[PATCH 1 of 2 shelve-ext] shelve: move node-pruning functionality to be member of shelvedstate

Pierre-Yves David pierre-yves.david at ens-lyon.org
Fri Apr 7 10:01:58 EDT 2017



On 04/07/2017 03:58 PM, Ryan McElroy wrote:
> On 4/7/17 2:46 PM, Kostia Balytskyi wrote:
>> # HG changeset patch
>> # User Kostia Balytskyi <ikostia at fb.com>
>> # Date 1491570726 25200
>> #      Fri Apr 07 06:12:06 2017 -0700
>> # Node ID c70f98920d904c94abe17f0b4fd141928dfa3e84
>> # Parent  f6d77af84ef3e936b15634759df2718d5363b78a
>> shelve: move node-pruning functionality to be member of shelvedstate
>>
>> This is just a piece of refactoring that I'd like to get in. It seems
>> harmless to me and will still be valualbe in future, when better
>> hiding mechanism is introduced.
>
> I agree with the direction of the cleanup.

I agree with the direction too but using "prune" might get confusing 
here. I would recommend using another word without an obsolete markers 
connotation.

For example cleanupnodes seems fine

What do you think?

>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -234,6 +234,11 @@ class shelvedstate(object):
>>       def clear(cls, repo):
>>           repo.vfs.unlinkpath(cls._filename, ignoremissing=True)
>>   +    def prunenodes(self, ui, repo):
>> +        """Cleanup temporary nodes from the repo"""
>> +        repair.strip(ui, repo, self.nodestoprune, backup=False,
>> +                     topic='shelve')
>> +
>
> "prune" is definitely a confusing name to use here. If the goal is to be
> "agnostic" to the type of node removal going on, call it "removenodes".
> If you want to be maximally clear, just call it "stripnodes" and rename
> it later when there's an alternate.
>
>>   def cleanupoldbackups(repo):
>>       vfs = vfsmod.vfs(repo.vfs.join(backupdir))
>>       maxbackups = repo.ui.configint('shelve', 'maxbackups', 10)
>> @@ -583,8 +588,7 @@ def unshelveabort(ui, repo, state, opts)
>>                   raise
>>                 mergefiles(ui, repo, state.wctx, state.pendingctx)
>> -            repair.strip(ui, repo, state.nodestoprune, backup=False,
>> -                         topic='shelve')
>> +            state.prunenodes(ui, repo)
>>           finally:
>>               shelvedstate.clear(repo)
>>               ui.warn(_("unshelve of '%s' aborted\n") % state.name)
>> @@ -655,7 +659,7 @@ def unshelvecontinue(ui, repo, state, op
>>           mergefiles(ui, repo, state.wctx, shelvectx)
>>           restorebranch(ui, repo, state.branchtorestore)
>>   -        repair.strip(ui, repo, state.nodestoprune, backup=False,
>> topic='shelve')
>> +        state.prunenodes(ui, repo)
>>           _restoreactivebookmark(repo, state.activebookmark)
>>           shelvedstate.clear(repo)
>>           unshelvecleanup(ui, repo, state.name, opts)
>>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list