Attention is currently required from: dexter.
Patch set 6:Code-Review -1
5 comments:
Patchset:
I think what we see from the several last few iterations of this patchset is that there is a lack of testing. I argue the code of patchset 5 and 6 are broken - yet we do not detect this in unit tests. Please add some tests that exercise those code paths.
File pySim/filesystem.py:
Patch Set #6, Line 746: def __get_size(self, total_len: Optional[int] = None) -> int:
must be `-> Optional[int]` as it also may return `None`
Patch Set #6, Line 1054: def __get_rec_len(self, total_len: Optional[int] = None) -> int:
likewise: `-> Optional[int]`
here we still don't return an Optional[int] but a dict? I think that's inconsistent comparing the __get_size situation above with the one here for record oriented EF?
and this will again break. Your __get_rec_len() method wil return a dict and you're prefixing that with total_len, so the result is something like `{'total_len': {'total_len': 23}`
To view, visit change 38195. To unsubscribe, or for help writing mail filters, visit settings.