Attention is currently required from: dexter.

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

Change subject: global_platform: refactor gen_install_parameters()
......................................................................


Patch Set 4:

(4 comments)

File pySim/global_platform/__init__.py:

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


File pySim/global_platform/install_param.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/7c197ba7_3a7ad2ba?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'. […]
You're checking each parameter individually anyway, so this top-level 
if-statement is not needed. It's redundant, so I am removing it in this patch.


https://gerrit.osmocom.org/c/pysim/+/42378/comment/e1c76a65_c1199d38?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
Well, this is literally what the code does:

* `if system_specific_params:`
 * If system specific parameters are present,
* `install_params_dict.append(...)`
 * add them to the install parameters.

Do we really need that level of verbosity in comments?


File tests/unittests/test_globalplatform.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/1cf491e1_af6f8640?usp=email :
PS3, Line 298:         load_parameters = gen_install_parameters()
> interesting that this does not change in behavior.
Not sure what you mean where, but let me explain why I am changing this line.

An empty string is not `None`, so `gen_install_parameters()` appends a single 
empty parameter. This causes the unit test to fail, so this is why I am 
removing the arguments here (this way they all become `None`).



--
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: 4
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Wed, 18 Mar 2026 08:40:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: dexter <[email protected]>

Reply via email to