Attention is currently required from: fixeria, laforge.
9 comments:
File contrib/saip-tool.py:
Patch Set #1, Line 22: import zipfile
Acknowledged
Yes and no. I have split the patchset up now to make it easier to read.
Patch Set #1, Line 250: if dictionary is None:
The type hint `:dict` suggests that it cannot be `None`, change to `Optional[dict]` then?
Done
elif value is None:
Looks like a no-op branch that can be removed without any consequences?
Not really, but there is a bug. If value is none, then we must not try to hexdump value. I forgot a return statement. Maybe it is more clear now.
Patch Set #1, Line 267: app_index = 0
shouldn't we simply make this part of the 'info' command? We already display some basic information […]
When I was using the list-apps sub-command I was happy to see only the application related output and not any additional information. So I think that it is a good idea to have a --apps parameter that only prints the application related output while hiding the basic info. This is also good when we add more PE specific info commands in the future.
Patch Set #1, Line 268: for app_pe in apps:
You can drop `app_index` above and use `enumerate()` instead: […]
Thanks, I didn't know that before.
I think this line should be removed, it's likely just a debug aid?
Done
Patch Set #1, Line 283: for inst in app_pe.decoded.get('instanceList', []):
Likewise, use `enumerate()` here.
Done
There can be multiple security domains. It's probably unusual, but legal AFAIR. […]
Done
yes, I think a method would be better style.
Done
To view, visit change 39860. To unsubscribe, or for help writing mail filters, visit settings.