This is an archive of the discontinued Mercurial Phabricator instance.

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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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.Sep 23 2020, 1:28 PM
pulkit added a subscriber: pulkit.Sep 25 2020, 3:18 AM
pulkit added inline comments.
mercurial/merge.py
2137

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.Sep 25 2020, 10:51 AM
martinvonz updated this revision to Diff 22853.
martinvonz added inline comments.Sep 25 2020, 10:51 AM
mercurial/merge.py
2137

Makes sense. Done.

pulkit accepted this revision.Sep 26 2020, 6:25 AM
This revision is now accepted and ready to land.Sep 26 2020, 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?