Attention is currently required from: fixeria.

dexter has posted comments on this change by fixeria. ( 
https://gerrit.osmocom.org/c/pysim/+/42378?usp=email )

Change subject: global_platform: make --install-parameters-* independently 
optional
......................................................................


Patch Set 3: Code-Review-1

(6 comments)

Patchset:

PS3:
This patch looks correct to me but it needs polishing.


File pySim/global_platform/__init__.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/22e1f0a3_9b8a25df?usp=email :
PS3, Line 901:                 # `--install-parameters-*` are all optional
Better fix this in the patch before.


File pySim/global_platform/install_param.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/d00b8813_e4d76196?usp=email :
PS3, Line 58:     if non_volatile_memory_quota and volatile_memory_quota and 
stk_parameter:
I think the error here is that it should be 'or' instead of 'and'. Only then 
the following code makes sense.


https://gerrit.osmocom.org/c/pysim/+/42378/comment/b9231988_b76b76bc?usp=email :
PS3, Line 60:
this looks correct, but I would add some comments to make the code easier to 
understand:

# collect system specific parameters (optional)


https://gerrit.osmocom.org/c/pysim/+/42378/comment/5e06b89d_f4093258?usp=email :
PS3, Line 67:         system_specific_params.append({'stk_parameter': 
stk_parameter})
# If system specific parameters are present, add them to the install parameters


File tests/unittests/test_globalplatform.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/c798f8f3_49d13fda?usp=email :
PS3, Line 298:         load_parameters = gen_install_parameters()
interesting that this does not change in behavior.



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/42378?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I8756fb38016cdf0527fe2e21edb44381d1dc557f
Gerrit-Change-Number: 42378
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Tue, 17 Mar 2026 15:25:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Reply via email to