D6484: states: unified api for checking unfinished states(gsoc-19)

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Thu Jun 6 00:18:59 EDT 2019


martinvonz requested changes to this revision.
martinvonz added a comment.
This revision now requires changes to proceed.


  Thanks for working on this!
  
  I think this patch does too much at once. At least the help message change should be moved to a separate patch. Ideally, the other changes would also be split up. Perhaps something like this:
  
  1. Move `unfinishedstates`, `checkunfinished`, and `clearunfinished` (without changing them) to the new module.
  2. Create and use the `statecheck` class, but only let it handle what's needed for `unfinishedstates` so far.
  3. Add the functionality from `STATES` to the new module and update the already existing users, but leave the cases that are handled by `STATES` unchanged for now.
  4. Switch existing users of `STATES` over to the new module (and delete `STATES`)
  5. Change messages to the new "To continue: ..." form. I'm not sure we want that change, or we'll probably at least want to make some, so this one is extra important to keep separate.
  
  You don't have to follow those steps exactly, of course. You know the code better, so maybe some of what I suggested doesn't make sense.

INLINE COMMENTS

> rebase.py:1951
> +    statecheckmod.unfinishedstates.append(statecheckmod.statecheck('rebase',
> +    'rebasestate', False, False, False))
>      cmdutil.afterresolvedstates.append(

Pass at least the boolean arguments as named arguments (`clearable=False` etc)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6484

To: taapas1128, durin42, martinvonz, #hg-reviewers
Cc: mjpieters, Kwan, mercurial-devel


More information about the Mercurial-devel mailing list