Page MenuHomePhabricator

upgrade: move `printrequirements()` to UpgradeOperation class
ClosedPublic

Authored by pulkit on Dec 14 2020, 4:55 AM.

Details

Summary

Part of refactor where we make things more arranged and integrated into single
UpgradeOperation class.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Dec 14 2020, 4:55 AM
marmoute added inline comments.
mercurial/upgrade_utils/actions.py
574–585

should we put this into _ attribute to avoid clutting the "public" API of the object ? (and maybe compute them within the print_requirements method?

pulkit added inline comments.Dec 28 2020, 3:36 AM
mercurial/upgrade_utils/actions.py
574–585

should we put this into _ attribute to avoid clutting the "public" API of the object ?

Agreed.

(and maybe compute them within the print_requirements method?

Maybe they can be used at more places, hence leaving them in __init__() for now.

pulkit updated this revision to Diff 24537.Dec 30 2020, 5:30 AM
marmoute added inline comments.Jan 5 2021, 2:52 AM
mercurial/upgrade_utils/actions.py
574–585

I am more leaning toward moving them in init when the "maybe" become a certitude. Or do you have actual code using that down the line ?

mharbison72 accepted this revision.Jan 5 2021, 10:34 AM
This revision is now accepted and ready to land.Jan 5 2021, 10:34 AM