D6876: phabricator: support automatically obsoleting old revisions of pulled commits
marmoute (Pierre-Yves David)
phabricator at mercurial-scm.org
Mon Oct 7 20:43:05 EDT 2019
marmoute added a comment.
The feature seens pretty usful, but is also a potential foot-gun/data-loss engine. I think it is useful to take the feature, but maybe with proper documentaiton warning and turned of by efaut. I made a couple of comment about the implementation.
> + if m:
> + return b"D%s" % m.group(r'id')
So, this only checks the diff number, right ? So in theory, we could obsolete a later version of this, silently dropping change (instead of detecting potential phase-divergence)
> + at eh.wrapcommand(b"pull")
> +def _pull(orig, ui, repo, *args, **opts):
> + if not (obsolete.isenabled(repo, obsolete.createmarkersopt)
It would probably make sense to wrap the pull exchange logic instead. It would catch more instance and could reuse the same transaction.
> + unfiltered = repo.unfiltered()
> + for rev in unfiltered.revs("draft() - obsolete()"):
> + n = unfiltered[rev]
This should be `non public()` instead of `draft()` these could be draft.
> + with unfiltered.lock(), unfiltered.transaction('phabpullcreatemarkers'):
> + obsolete.createmarkers(unfiltered, tocreate)
We should do the computation before locking to avoid other process updating the repo under our feet.
CHANGES SINCE LAST ACTION
To: mharbison72, #hg-reviewers
Cc: pulkit, sheehan, Kwan, mercurial-devel, marmoute
More information about the Mercurial-devel