Page MenuHomeSoftware Heritage

hg/git: Align lister's output (load-hg/git tasks) with loader's new format
ClosedPublic

Authored by ardumont on Dec 6 2019, 5:36 PM.

Details

Diff Detail

Repository
rDLS Listers
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Dec 6 2019, 5:36 PM
anlambert added inline comments.
swh/lister/bitbucket/lister.py
36–53

I think you should rather call the utils.create_task_dict function with keyword arguments directly in the task_dict method from the ListerBase class instead of overriding it here.

37

s/Mapping/Dict/

ardumont updated this revision to Diff 8518.Dec 9 2019, 10:02 AM

Adapt according to review

swh/lister/bitbucket/lister.py
36–53

Yes. Adapting right now.

Relatedly, i also think i need to align the loader-git accordingly... D2409

ardumont retitled this revision from bitbucket: Align lister's loader output tasks with expected format to hg: Align lister's output (load-hg tasks) with loader's expected format.Dec 9 2019, 10:17 AM
anlambert added inline comments.Dec 9 2019, 11:57 AM
swh/lister/core/lister_base.py
394–395

I would rather use the same task arguments format (keyword) for all loaders here.

ardumont added inline comments.Dec 9 2019, 2:13 PM
swh/lister/core/lister_base.py
394–395

Yes, same.

i'll land D2409 and D2410 first ;)

ardumont retitled this revision from hg: Align lister's output (load-hg tasks) with loader's expected format to hg/git: Align lister's output (load-hg/git tasks) with loader's new format.Dec 9 2019, 2:17 PM
ardumont planned changes to this revision.Dec 9 2019, 2:21 PM
ardumont updated this revision to Diff 8526.Dec 9 2019, 2:49 PM

hg/git: Align lister's output (load-hg/git tasks) with loader's new format

ardumont added inline comments.Dec 9 2019, 2:50 PM
swh/lister/core/lister_base.py
394–395

I could not really do it before (well morally).

anlambert accepted this revision.Dec 9 2019, 2:51 PM

Looks good !

swh/lister/core/lister_base.py
380

s/Mapping/Dict/

This revision is now accepted and ready to land.Dec 9 2019, 2:51 PM
ardumont added inline comments.Dec 9 2019, 2:53 PM
swh/lister/core/lister_base.py
380

Something hit me, thus i keep doing that.

I thought it was better to be generic in the output (so Mapping here)
And specific in the input (so Dict as parameter).

That's the one thing i remember clearly from zack's slide (and what he told me ;).
Am i misremembering?

anlambert added inline comments.Dec 9 2019, 3:05 PM
swh/lister/core/lister_base.py
380

Yes, the recommended way is the opposite: use a concrete type in the output and a generic type in the input.

See https://mypy.readthedocs.io/en/latest/cheat_sheet_py3.html#standard-duck-types

ardumont added inline comments.Dec 9 2019, 3:10 PM
swh/lister/core/lister_base.py
380

Thanks!

Will try to fix my erroneous ways ;)

ardumont updated this revision to Diff 8527.Dec 9 2019, 3:13 PM

Fix typing

ardumont added inline comments.Dec 9 2019, 3:22 PM
swh/lister/core/lister_base.py
380

I checked zack's slide.
I inverted what's said there, great!

Be conservative in what you send, be liberal in what you accept.