( )⚙ D8799 merge: return a mergeresult obj from manifestmerge(), calculateupdates() (API)

This is an archive of the discontinued Mercurial Phabricator instance.

merge: return a mergeresult obj from manifestmerge(), calculateupdates() (API)
ClosedPublic

Authored by pulkit on Jul 23 2020, 10:11 AM.

Details

Summary

Earlier, manifestmerge() and calculateupdates() returns a tuple of three things.
I wanted to add one more thing to return value.

Introducing a special class which represents results of a merge will help
understand better and also ease adding new return values.

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.Jul 23 2020, 10:11 AM
pulkit retitled this revision from merge: return new mergeresult obj from manifestmerge(),calculateupdates() (API) to merge: return a mergeresult obj from manifestmerge(), calculateupdates() (API).Jul 25 2020, 5:56 AM
pulkit updated this revision to Diff 22096.
pulkit added a comment.EditedJul 25 2020, 6:08 AM

This whole series aims to improve the merge code by introducing a mergeresult object which represents result of a merge and using it in various merge related functions. Right now all of this done using actions dict which is also different in different functions. Having a dedicated object will make things easier to understand and do some optimizations.

The series however long consists of small obvious patches.

indygreg accepted this revision.Aug 2 2020, 12:46 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/merge.py
563

I'm generally not a fan of having trivial @property wrappers for internal attributes like this, as I don't think it adds any value. Especially since there is code mutating the internal state, which is a bit janky.

But I'll approve this anyway. If the code remains unchanged after the completion of the series, my vote is to remove the @property wrappers and access attributes directly.

This revision is now accepted and ready to land.Aug 2 2020, 12:46 PM