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=email : 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=email : 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=email : PS6, Line 598: -> bool: likewise, change to `-> None`? https://gerrit.osmocom.org/c/pysim/+/38049/comment/01f92e4c_a4b824e1?usp=email : 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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <[email protected]> Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: dexter <[email protected]> Gerrit-Comment-Date: Mon, 16 Sep 2024 08:14:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
