Details
- Reviewers
ardumont - Group Reviewers
Reviewers - Commits
- rDLDBASE592156740883: nixguix: validate and clean sources.json structure
Diff Detail
- Repository
- rDLDBASE Generic VCS/Package Loader
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 11596 Build 17583: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 17582: arc lint + arc unit
Event Timeline
Build is green
Patch application report for D2951 (id=10505)
Rebasing onto fbf457e92c...
Current branch diff-target is up to date.
Changes applied before test
commit 3cb03cf17ffd3962b042c1390e50ddf2e95b881a Author: Antoine Eiche <lewo@abesis.fr> Date: Fri Apr 3 10:34:41 2020 +0200 nixguix: validate and clean sources.json structure
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/8/ for more details.
I have read it again.
There are some typos to fix and possible improvments to do ;)
In the end, let's improve a bit ;)
swh/loader/package/nixguix/loader.py | ||
---|---|---|
49 | must contain ;) | |
50 | Make it more generic, something like: missing_keys = [] for required_key in requires_keys: if required_key not in sources: missing_keys.append(required_key) if missing_keys: raise ValueError("sources structure invalid, missing: %s", ",".join(missing_keys)) # you can also give the full expected list... or something similar | |
73 | curious me, what's the difference with not isinstance(source['urls'], list)? | |
swh/loader/package/nixguix/tests/test_nixguix.py | ||
38 | you can add: with pytest.raises(ValueError, match="sources structure must contain"): ... | |
49 | Add the match="version '2' not supported"... |
Addressed @ardumont comments. Thanks for your review!
swh/loader/package/nixguix/loader.py | ||
---|---|---|
73 | isinstance also works on indirect instance of list while my implementation only worked on list itself. But I think isinstance is better than my tricks:/ So, i switched to isinstance. | |
swh/loader/package/nixguix/tests/test_nixguix.py | ||
38 | Oh, nice. I didn't know that! |
Build is green
Patch application report for D2951 (id=10519)
Rebasing onto fbf457e92c...
Current branch diff-target is up to date.
Changes applied before test
commit 592156740883fddd8c4d1d8dc79005cf5b5a1d0a Author: Antoine Eiche <lewo@abesis.fr> Date: Fri Apr 3 10:34:41 2020 +0200 nixguix: validate and clean sources.json structure
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/9/ for more details.