[PATCH 1 of 2] strip: add a delayedstrip method that works in a transaction

Jun Wu quark at fb.com
Tue Jun 27 00:23:23 EDT 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