D6501: state: created new class statecheck to handle unfinishedstates
pulkit (Pulkit Goyal)
phabricator at mercurial-scm.org
Mon Jun 10 19:02:19 EDT 2019
pulkit added inline comments.
INLINE COMMENTS
> histedit.py:2291
> cmdutil.summaryhooks.add('histedit', summaryhook)
> - statemod.unfinishedstates.append(
> - ['histedit-state', False, True, _('histedit in progress'),
> - _("use 'hg histedit --continue' or 'hg histedit --abort'")])
> + statemod.unfinishedstates.append(statemod.statecheck('histedit',
> + 'histedit-state', clearable=False, allowcommit=True))
the code around these statements doing `append` is looking a bit complex. for making it cleaner and more readable, let's do either one of thing:
1. initialize an object before and then append
2. improve formatting of this
This applies to all the `append` which are there in the patch.
> state.py:91
>
> -# A list of state files kept by multistep operations like graft.
> -# Since graft cannot be aborted, it is considered 'clearable' by update.
this comment on top of unfinished state is lost. We should reword and keep it.
> state.py:92
> +class statecheck(object):
> + """a utility class that will to deal with multistep operations
> + like graft, histedit, bisect, update etc and check whether such commands
`that will to deal` -> `that deals`
> state.py:116
> + """
> + self.cmdname = cmdname
> + self.fname = fname
prepend all the class names with an `_`, like `self._cmdname`, `self._fname` etc. only the variables
That's a code style to say they are internal to readers and class users.
> state.py:140
> +
> +unfinishedstates=[]
> +unfinishedstates.append(statecheck('graft', 'graftstate', clearable=True,
needs space between `=` operator. I guess this is caught by `test-check*`, make sure you run tests.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D6501
To: taapas1128, durin42, martinvonz, #hg-reviewers
Cc: pulkit, mjpieters, mercurial-devel
More information about the Mercurial-devel
mailing list