D6503: statecheck: added support for STATES

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Wed Jun 12 14:24:40 EDT 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