D529: uncommit: move fb-extension to core which uncommits a changeset
durham (Durham Goode)
phabricator at mercurial-scm.org
Mon Sep 11 16:23:15 UTC 2017
durham accepted this revision.
durham added a comment.
Looks good, except for my ui.allowemptycommit comment.
INLINE COMMENTS
> uncommit.py:59
> + files = (initialfiles - exclude)
> + allowempty = allowempty or repo.ui.configbool('ui', 'allowemptycommit')
> + if not files and not allowempty:
If I read this right, it means a user having ui.allowemptycommit set to True will cause hg uncommit to always leave a commit behind? I don't think that's what we want. ui.allowemptycommit is to let a user make an empt commit if they want. It's not really meant to say 'leave empty commits behind for me'. I don't think it should affect the uncommit command.
I see this came from our original extension. I'd still drop it.
> uncommit.py:61
> + if not files and not allowempty:
> + return ctx.parents()[0].node()
> +
Might be worth a comment that returning p1 here is special cased to not create an obsmarker later.
> uncommit.py:154
> + if not old.mutable():
> + raise error.Abort(_('cannot amend public changesets'))
> + if len(old.parents()) > 1:
Should "amend" be "uncommit" here?
> uncommit.py:159
> + if not allowunstable and old.children():
> + raise error.Abort(_('cannot amend changeset with children'))
> +
Same here, "amend" -> "uncommit"
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D529
To: pulkit, #hg-reviewers, quark, durham
Cc: durham, quark, martinvonz, yuja, mercurial-devel
More information about the Mercurial-devel
mailing list