Attention is currently required from: dexter, laforge.

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

Change subject: global_platform: LOAD and INSTALL [for load] support
......................................................................


Patch Set 7:

(6 comments)

File pySim/global_platform/__init__.py:

https://gerrit.osmocom.org/c/pysim/+/37454/comment/9e121e2d_cb2e3367?usp=email :
PS7, Line 20: import io
unused import?
(hm, why doesn't the linter complain?)


https://gerrit.osmocom.org/c/pysim/+/37454/comment/622c3a16_f8831d53?usp=email :
PS7, Line 772: Prefixed(Int8ub, GreedyBytes
This is basically LV structure. Given that it's used several times and can be 
useful in other places, maybe define it somewhere? Wait, actually it's already 
defined in pyosmocom:

```
# here we collect some shared / common definitions of data types
LV = Prefixed(Int8ub, HexAdapter(GreedyBytes))
```

It's a bit different, but I assume it works the same way?


https://gerrit.osmocom.org/c/pysim/+/37454/comment/9020f333_a7771748?usp=email :
PS7, Line 824: ('--from-hex
why not using `type=is_hexstr` here?


https://gerrit.osmocom.org/c/pysim/+/37454/comment/391df02d_45717cef?usp=email :
PS7, Line 824:         load_parser.add_argument('--from-hex', help='load from 
hex string')
             :         load_parser.add_argument('--from-file', help='load from 
binary file')
             :
If these params are mutually-exclusive (only one of these must be supplied), 
please create a group using `add_mutually_exclusive_group()` and make that 
group `mandatory=True`.


https://gerrit.osmocom.org/c/pysim/+/37454/comment/e066d0cc_23d5c223?usp=email :
PS7, Line 833:
formatting


https://gerrit.osmocom.org/c/pysim/+/37454/comment/5ededfac_4e9b9779?usp=email :
PS7, Line 835: with open(opts.from_file, 'rb') as f:
BTW, argparse does have: 
https://docs.python.org/3/library/argparse.html#filetype-objects



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/37454?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: I924aaeecbb3a72bdb65eefbff6135e4e9570579e
Gerrit-Change-Number: 37454
Gerrit-PatchSet: 7
Gerrit-Owner: laforge <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: dexter <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Mon, 20 Jan 2025 12:13:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Reply via email to