Attention is currently required from: fixeria, laforge.
dexter has posted comments on this change by dexter. (
https://gerrit.osmocom.org/c/pysim/+/37838?usp=email )
Change subject: pySim-shell: fix CardKeyProvider for chv management commands
......................................................................
Patch Set 4:
(2 comments)
File pySim-shell.py:
https://gerrit.osmocom.org/c/pysim/+/37838/comment/d212fe9a_e72623be?usp=em… :
PS3, Line 868: verify_chv_parser.add_argument('PIN', nargs='?',
type=is_decimal,
Why are you moving positional arguments before the
optional `--pin-nr`? It's not critical, but makes […]
Its the same with verify
ADM. I have changed it so that it is consistent with verify_adm. I guess this means I have
to go through all parser definitions to see if it is wrong elsewhere.
https://gerrit.osmocom.org/c/pysim/+/37838/comment/6743a4f6_818f9fea?usp=em… :
PS3, Line 902: change_chv_parser.add_argument('NEWPIN', nargs='?',
type=is_decimal,
You're breaking compatibility by changing the
ordering here: currently it's `OLD NEW`, but your patc […]
This is intentional.
Since we no longer use the placeholders, we must put NEW as first positional parameter so
that we can leave OLD out to in case we want to get OLD from the CardKeyProvider.
--
To view, visit
https://gerrit.osmocom.org/c/pysim/+/37838?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: I565b56ac608e801c67ca53d337bdec9efa3f3817
Gerrit-Change-Number: 37838
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 22 Aug 2024 10:00:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>