This actually migrates the loader svn to the latest swh.model api change.
This also fixes the current ci build failure [1]
[1] https://jenkins.softwareheritage.org/job/DLDSVN/job/tests/540/
Details
Details
- Reviewers
olasd - Group Reviewers
Reviewers - Commits
- rDLDSVN7340046eaa10: loader.svn: Use swh.model objects within the loader
tox
Diff Detail
Diff Detail
- Repository
- rDLDSVN Subversion (SVN) loader
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DLDSVN/job/tox/69/ for more details.
Comment Actions
There's a few comments that aren't really related to this diff but to potential improvements of swh.model. Overall the changes look sensible to me.
| swh/loader/svn/converters.py | ||
|---|---|---|
| 37 | This whole function should probably be lifted to swh.model at some point, because it's popping up everywhere | |
| swh/loader/svn/ra.py | ||
| 454–472 | There was a simplicity to the .collect() operation that we've lost in that refactoring. This kind of loop is going to pop up everywhere as well, and we'll probably want to lift it up. | |
| swh/loader/svn/svn.py | ||
| 224 | What does gens mean? | |
| swh/loader/svn/utils.py | ||
| 30 | Can we move that to iso8601? | |
Comment Actions
- Rename gens to objects, it's clearer
- Add TODO according to olasd's good points
| swh/loader/svn/converters.py | ||
|---|---|---|
| 37 | some form of it, i'm not entirely sure the implementation is the same everywhere. But indeed, we should have a sane implementation as a library helper somewhere, maybe swh.model as you proposed. | |
| swh/loader/svn/ra.py | ||
| 454–472 | Yes, agreed. | |
| swh/loader/svn/svn.py | ||
| 224 | badly named, i should have kepts objects (even though the type changed from dict to tuple). I meant it as generators but it's wrong, i failed to see it earlier. Thanks! | |
| swh/loader/svn/utils.py | ||
| 30 | Yes, but i'm pretty sure the input is not always iso8601... | |
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DLDSVN/job/tox/70/ for more details.
| swh/loader/svn/utils.py | ||
|---|---|---|
| 30 | The tests you've introduced, and the documentation of this function, are all iso8601, hence my comment :) | |