Page MenuHomePhabricator

merge: add a higher-level update() for the common `hg update` use case
ClosedPublic

Authored by martinvonz on Sep 21 2020, 4:36 PM.

Details

Summary

This is different from the update() function that I just made
private. The new function is specifically for the normal hg update
use case. It doesn't do a merge and it doesn't do a clean (forced)
update.

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

martinvonz created this revision.Sep 21 2020, 4:36 PM
martinvonz updated this revision to Diff 22768.Sep 23 2020, 3:12 AM
martinvonz updated this revision to Diff 22807.Wed, Sep 23, 1:28 PM
pulkit added a subscriber: pulkit.Fri, Sep 25, 3:18 AM
pulkit added inline comments.
mercurial/merge.py
2060

Let's add couple of lines of documentation here that if you are looking for old merge.update(), merge._update() is the thing you are looking for.

martinvonz marked an inline comment as done.Fri, Sep 25, 10:51 AM
martinvonz updated this revision to Diff 22853.
martinvonz added inline comments.Fri, Sep 25, 10:51 AM
mercurial/merge.py
2060

Makes sense. Done.

pulkit accepted this revision.Sat, Sep 26, 6:25 AM
This revision is now accepted and ready to land.Sat, Sep 26, 6:25 AM

[Moving a discussion we are having on #hg-evolve here to reach more people]

The clarification of interface seems great overall. However I am a bit
worried about the drop of the initial repo argument to these function.

Passing the repository explicitly seems safer to me. It make sure we are
updating the right repository, and more importantly with the right
repoview (filtering level). It would be easy for some fault code to
reuse output from a function using relaxed filtering to use in a more
filtered context. explicitly passing the repository catch that.

What do you think ?

[Moving a discussion we are having on #hg-evolve here to reach more people]
The clarification of interface seems great overall. However I am a bit
worried about the drop of the initial repo argument to these function.
Passing the repository explicitly seems safer to me. It make sure we are
updating the right repository, and more importantly with the right
repoview (filtering level). It would be easy for some fault code to
reuse output from a function using relaxed filtering to use in a more
filtered context. explicitly passing the repository catch that.
What do you think ?

I can see the concern at some abstract level, but I don't know what you mean more concretely. What would such bad callers look like? Also, does your concern apply to most places we pass around context objects?