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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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