Attention is currently required from: dexter, laforge.
fixeria has posted comments on this change by laforge. (
https://gerrit.osmocom.org/c/pysim/+/38049?usp=email )
Change subject: pySim-shell: Detect different eUICC types and print during start-up
......................................................................
Patch Set 6:
(4 comments)
File pySim/euicc.py:
https://gerrit.osmocom.org/c/pysim/+/38049/comment/3c597a2e_3663e855?usp=em… :
PS6, Line 580: -> bool:
This function does not return a `bool`, but always `None` (implicit return) AFAICS.
https://gerrit.osmocom.org/c/pysim/+/38049/comment/8f9cd7f0_b497fca6?usp=em… :
PS6, Line 586: @staticmethod
(just some ideas below, not blocking)
Maybe use `@classmethod` instead, so that you can do:
```
@classmethod
def match_with_card(cls, scc: SimCardCommands) -> bool:
return match_helper(scc, cls._match_with_card)
```
Pros: no need to specify the class name and thus less risk of copy-paste errors.
This method can then go to the parent class `CardProfileUICC` to avoid duplication.
The `_match_with_card()` can then become an abstract static method.
https://gerrit.osmocom.org/c/pysim/+/38049/comment/fff61958_19c6526e?usp=em… :
PS6, Line 598: -> bool:
likewise, change to `-> None`?
https://gerrit.osmocom.org/c/pysim/+/38049/comment/01f92e4c_a4b824e1?usp=em… :
PS6, Line 616: -> bool:
likewise, change to `-> None`?
--
To view, visit
https://gerrit.osmocom.org/c/pysim/+/38049?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: I54ea4ce663693f3951040dcc8a16bf532bf99c02
Gerrit-Change-Number: 38049
Gerrit-PatchSet: 6
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 16 Sep 2024 08:14:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No