Page MenuHomeSoftware Heritage

swh.lister.gitlab: Make the lister's task instance parametric
ClosedPublic

Authored by ardumont on Jul 6 2018, 4:17 PM.

Diff Detail

Repository
rDLS Listers
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1288
Build 1632: arc lint + arc unit

Event Timeline

olasd requested changes to this revision.Jul 6 2018, 5:27 PM
olasd added a subscriber: olasd.

I don't much like the pattern of implicit *args and **kwargs on the leaf tasks: it makes it harder to discover what the arguments are supposed to look like, and it pushes "bug discovery" further down the stack if we change the function signatures.

I'd almost suggest making the leaf GitLab tasks have explicit signatures for run() (that just call super().run) but that's a "bit" overkill.

I'm okay with the changes to the base classes.

swh/lister/bitbucket/tasks.py
13

That shouldn't be necessary.

swh/lister/debian/tasks.py
13

That shouldn't be necessary.

16

That shouldn't be necessary.

swh/lister/github/tasks.py
13

That shouldn't be necessary.

This revision now requires changes to proceed.Jul 6 2018, 5:27 PM
ardumont marked 3 inline comments as done.

Amended commit to remove unnecessary modification

olasd added inline comments.
swh/lister/gitlab/tasks.py
13–16

I guess this lister_name argument is not needed anymore :)

This revision is now accepted and ready to land.Jul 16 2018, 11:14 AM
ardumont added inline comments.
swh/lister/gitlab/tasks.py
13–16

Yes, and it's no longer there but i fear that updating the diff will mess it up.

This revision was automatically updated to reflect the committed changes.
ardumont marked an inline comment as done.