tox
Details
- Reviewers
vlorentz anlambert - Group Reviewers
Reviewers - Commits
- rDMOD363b1659a6f5: Unify object_type some more within the merkle and from_disk modules
Diff Detail
- Repository
- rDMOD Data model
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build is green
Patch application report for D3390 (id=12030)
Rebasing onto 40a40f508b...
Current branch diff-target is up to date.
Changes applied before test
commit 262d84043df0fb0c91082eb94cc91f4d2a8b9902 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Jul 2 10:58:43 2020 +0200 Unify object_type some more within the merkle and from_disk modules
See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/89/ for more details.
LGTM, maybe you could set object_type type as Final like in other model classes (see inline comment).
swh/model/merkle.py | ||
---|---|---|
113–114 | Maybe you should type this as Final like in swh.model.model and set some generic object_type value for base merkle model classes ("merkle_node", "merkle_leaf" ?) Also, maybe the TODO could be removed ? |
swh/model/merkle.py | ||
---|---|---|
113–114 | I'll try, I did not in the first place as my understanding of the Final would not work here (base class, etc...). I left it because I did not really understand it but why not ;) |
kinda hope that solves [1]
lazy bones!
[1] https://sentry.softwareheritage.org/share/issue/594af974da3c482b8419cda22a21becc
In the end, that may not be the issue (and it's for the best otherwise, it would have only hidden the bug)
It'd be more like old loaders not properly stopped [2] still running with old code...
I believe the diff is still worth unification though, less to think about for later
[2] P707
swh/model/merkle.py | ||
---|---|---|
113–114 |
Oh right, this declare the attribute read only. Maybe remove it from the abstract base class then ? |
swh/model/merkle.py | ||
---|---|---|
113–114 |
it being the todo ;)
i'll try, i'm not so sure mypy will be happy around the lines below then though. |
Build is green
Patch application report for D3390 (id=12039)
Rebasing onto 40a40f508b...
Current branch diff-target is up to date.
Changes applied before test
commit 363b1659a6f5fbc32472f0774352d58d068f4282 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Jul 2 10:58:43 2020 +0200 Unify object_type some more within the merkle and from_disk modules
See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/92/ for more details.