[PATCH 4 of 4 shelve-ext v2] shelve: rename nodestoprune to nodestoremove

Ryan McElroy rm at fb.com
Mon Apr 10 15:05:11 EDT 2017


On 4/10/17 5:04 PM, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1491839131 25200
> #      Mon Apr 10 08:45:31 2017 -0700
> # Node ID 52a03106bbabb94fb62d69d4a3eb76ef9614c134
> # Parent  5c5d69830d176d7eb096c6ccb2f72377e13ace97
> shelve: rename nodestoprune to nodestoremove

We really should rename this before introducing the key-value state 
file. Can you change the name after patch 1? Then there will never be a 
code point where the "broken" name is intserted into a statefile.

Overall, the direction of these patches makes sense, but there are 
enough little issues that I'll mark these as "changes requested" in 
patchwork.

In a v3, I'd like to see:

* typos cleaned up
* rename "prune" to "remove" before introducing key-value statefile
* "st" renamed to "state"

The rest was just random questions I had along the way.

This looks like a good improvement overall, I'm looking forward to v3.

>
> As per feedback from the community.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -194,7 +194,7 @@ class shelvedstate(object):
>       def load(cls, repo):
>           # order is important, please do not change
>           keys = ['version', 'name', 'originalwctx', 'pendingctx', 'parents',
> -                'nodestoprune', 'branchtorestore', 'keep', 'activebook']
> +                'nodestoremove', 'branchtorestore', 'keep', 'activebook']
>           st = {}
>           if cls.iskeyvaluebased(repo):
>               st = scmutil.simplekeyvaluefile(repo.vfs, cls._filename).read()
> @@ -216,8 +216,8 @@ class shelvedstate(object):
>               st['pendingctx'] = nodemod.bin(st.get('pendingctx'))
>               st['parents'] = [nodemod.bin(h)
>                                for h in st.get('parents').split(' ')]
> -            st['nodestoprune'] = [nodemod.bin(h)
> -                                  for h in st.get('nodestoprune').split(' ')]
> +            st['nodestoremove'] = [nodemod.bin(h)
> +                                  for h in st.get('nodestoremove').split(' ')]
>           except (ValueError, TypeError, AttributeError) as err:
>               raise error.CorruptedState(str(err))
>   
> @@ -227,7 +227,7 @@ class shelvedstate(object):
>               obj.wctx = repo[st.get('originalwctx')]
>               obj.pendingctx = repo[st.get('pendingctx')]
>               obj.parents = st.get('parents')
> -            obj.nodestoprune = st.get('nodestoprune')
> +            obj.nodestoremove = st.get('nodestoremove')
>               obj.branchtorestore = st.get('branchtorestore')
>               obj.keep = st.get('keep') == cls._keep
>               obj.activebookmark = ''
> @@ -239,7 +239,7 @@ class shelvedstate(object):
>           return obj
>   
>       @classmethod
> -    def save(cls, repo, name, originalwctx, pendingctx, nodestoprune,
> +    def save(cls, repo, name, originalwctx, pendingctx, nodestoremove,
>                branchtorestore, keep=False, activebook=''):
>           if not repo.ui.configbool('shelve', 'oldstatefile', False):
>               info = {
> @@ -249,8 +249,8 @@ class shelvedstate(object):
>                   "pendingctx": nodemod.hex(pendingctx.node()),
>                   "parents": ' '.join([nodemod.hex(p)
>                                        for p in repo.dirstate.parents()]),
> -                "nodestoprune": ' '.join([nodemod.hex(n)
> -                                          for n in nodestoprune]),
> +                "nodestoremove": ' '.join([nodemod.hex(n)
> +                                           for n in nodestoremove]),
>                   "branchtorestore": branchtorestore,
>                   "keep": cls._keep if keep else cls._nokeep,
>                   "activebook": activebook or cls._noactivebook
> @@ -265,7 +265,7 @@ class shelvedstate(object):
>           fp.write('%s\n' %
>                    ' '.join([nodemod.hex(p) for p in repo.dirstate.parents()]))
>           fp.write('%s\n' %
> -                 ' '.join([nodemod.hex(n) for n in nodestoprune]))
> +                 ' '.join([nodemod.hex(n) for n in nodestoremove]))
>           fp.write('%s\n' % branchtorestore)
>           fp.write('%s\n' % (cls._keep if keep else cls._nokeep))
>           fp.write('%s\n' % (activebook or cls._noactivebook))
> @@ -277,7 +277,7 @@ class shelvedstate(object):
>   
>       def removenodes(self, ui, repo):
>           """Cleanup temporary nodes from the repo"""
> -        repair.strip(ui, repo, self.nodestoprune, backup=False,
> +        repair.strip(ui, repo, self.nodestoremove, backup=False,
>                        topic='shelve')
>   
>   def cleanupoldbackups(repo):
> @@ -695,7 +695,7 @@ def unshelvecontinue(ui, repo, state, op
>               shelvectx = state.pendingctx
>           else:
>               # only strip the shelvectx if the rebase produced it
> -            state.nodestoprune.append(shelvectx.node())
> +            state.nodestoremove.append(shelvectx.node())
>   
>           mergefiles(ui, repo, state.wctx, shelvectx)
>           restorebranch(ui, repo, state.branchtorestore)
> @@ -753,9 +753,9 @@ def _rebaserestoredcommit(ui, repo, opts
>       except error.InterventionRequired:
>           tr.close()
>   
> -        nodestoprune = [repo.changelog.node(rev)
> -                        for rev in xrange(oldtiprev, len(repo))]
> -        shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoprune,
> +        nodestoremove = [repo.changelog.node(rev)
> +                         for rev in xrange(oldtiprev, len(repo))]
> +        shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoremove,
>                             branchtorestore, opts.get('keep'), activebookmark)
>   
>           repo.vfs.rename('rebasestate', 'unshelverebasestate')
>




More information about the Mercurial-devel mailing list