[PATCH 1 of 2] strip: add a delayedstrip method that works in a transaction
Jun Wu
quark at fb.com
Tue Jun 27 04:23:23 UTC 2017
Excerpts from Jun Wu's message of 2017-06-26 17:33:26 -0700:
> Handling backup=True/False will make the API more complex than desired. I
> don't really have a clean way to deal with that. Take the above example, it
> could also be:
>
> delayedstrip(nodelist=['A', 'D'], backup=True)
> delayedstrip(nodelist=['B', 'C'], backup=False)
>
> Therefore I don't think "delayedstrip" should replace "strip", the word
> "delayed" is there for a reason. But I agree it's nicer if "strip" does the
> orphan check. So the change will look like:
>
> def strip(....):
> roots = safestriproots(nodelist)
> _strip(roots)
>
> def _strip(....):
> # the original "strip"
After thinking it again, the "backup" parameter issue is not a big deal. We
can do something like "the last non-None value is respected".
And it seems there is no valid use cases where strip has to run explicitly
outside a transaction. So replacing "strip" makes sense to me.
I'll try to migrate the existing code to the new strip function. Thanks for
the idea!
That said, I still insist that strip warning is better than ProgrammingError
since test-rebase-interruptions.t has a case where the error maker is a user
running "hg commit", not a programmer :)
More information about the Mercurial-devel
mailing list