Page MenuHomePhabricator

treemanifest: store ui once instead of using repo.ui every time

Authored by singhsrb on Dec 15 2017, 3:33 PM.


Group Reviewers
Restricted Project

setuptreestores caters to a lot of configurations through the ui
instance. It makes sense to store the ui instance once instead of referring
to it through repo.ui every time.

Test Plan

Ran all the tests.

Diff Detail

rFBHGX Facebook Mercurial Extensions
Lint Skipped
Unit Tests Skipped

Event Timeline

singhsrb created this revision.Dec 15 2017, 3:33 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 15 2017, 3:33 PM
quark added a subscriber: quark.Dec 15 2017, 7:15 PM

I think the reason there are both ui and repo in @command interface is because some commands do not need a repo but need a ui. Or sometimes repo.ui is different from ui (in chg's case). In this case repo.ui is ui. It is cleaner to avoid two arguments.


Why not insert ui = repo.ui here?

singhsrb added inline comments.Dec 15 2017, 7:17 PM

Sure! that works as well! I wanted to be sure I am not missing something.

singhsrb edited the summary of this revision. (Show Details)Dec 15 2017, 7:24 PM
singhsrb retitled this revision from treemanifest: include ui in the method signature instead of using repo.ui to treemanifest: store ui once instead of using repo.ui every time.
singhsrb updated this revision to Diff 4522.
quark accepted this revision.Dec 15 2017, 7:28 PM
This revision is now accepted and ready to land.Dec 15 2017, 7:28 PM
singhsrb abandoned this revision.Jan 31 2018, 5:45 PM

Will be rebasing and floating for review again.