D6503: statecheck: added support for STATES
martinvonz (Martin von Zweigbergk)
phabricator at mercurial-scm.org
Wed Jun 12 18:24:40 UTC 2019
martinvonz added a comment.
In D6503#94684 <https://phab.mercurial-scm.org/D6503#94684>, @taapas1128 wrote:
> @martinvonz I haven't changed anything regarding rebase and update on interrupted states hence not added any tests.
Do those commands call checkunfinished?
INLINE COMMENTS
> taapas1128 wrote in state.py:161
> I will correct that None. Maybe an empty string `""` would serve better.
I think None is better. That more clearly says "there is no associated file", while empty string sounds more like "the associated file is named ''".
> taapas1128 wrote in state.py:161
> In `getrepostate()` function merge must come last that is why I have filtered off update and merge and then checked them at the very end. In `checkunfinished()` there is no such thing.
I know that's what you did regarding merge (as I said). I suggested a different solution (or three, actually). Are you saying that won't work? Can you explain why in that case?
> taapas1128 wrote in state.py:174
> Actually in older code too there is no support for merge and bisect in `checkunfinished()` or `clearunfinished()` and is handled seperately. So I have bypassed merge and bisect over here too to comply with it. As you can see later in `clearunfinished()` has `util.unlink()` and as fname is not present for merge it cant be passed here.
So what happens if we check merge state too here? I know we didn't do it before, but would it make sense to do it? It would probably belong in a separate patch if we did it.
> taapas1128 wrote in state.py:215
> These are hard-coded as we need to check update and merge last. Earlier they were hard coded in form of a list. here I have bypassed them earlier and then checked for them last .
I know. That's what I tried to explain. I suggested different solutions. Can you explain why they won't work or why they're worse?
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D6503/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D6503
To: taapas1128, durin42, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
More information about the Mercurial-devel
mailing list