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

2017-04-07 Thread Kostia Balytskyi 
I am fine with renaming nodestoprune/prunenodes, but not to cleanupnodes: there 
are already 14 lines with word "cleanup" in shelve.py. If you can come up with 
a better name, you can change it always (especially given that class member is 
already nodestoprune).

-Original Message-
From: Pierre-Yves David [mailto:pierre-yves.da...@ens-lyon.org] 
Sent: Friday, 7 April, 2017 15:02
To: Ryan McElroy ; Kostia Balytskyi ; 
mercurial-devel@mercurial-scm.org
Subject: Re: [PATCH 1 of 2 shelve-ext] shelve: move node-pruning functionality 
to be member of shelvedstate



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  # 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@mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Ds
> cm.org_mailman_listinfo_mercurial-2Ddevel=DwICaQ=5VD0RTtNlTh3ycd41
> b3MUw=Pp-gQYFgs4tKlSFPF5kfCw=sQIiGyuq_0IQmaF9PP8F0DCU2b0lP5av_lDck
> g3idtk=BSi9_RvTDzP8V3KckQYSrbpXpL2g6A85sCIcc2dID4w=

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

2017-04-07 Thread Pierre-Yves David 



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 
# 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@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

2017-04-07 Thread Ryan McElroy 

On 4/7/17 2:46 PM, Kostia Balytskyi wrote:

# HG changeset patch
# User Kostia Balytskyi 
# 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.



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@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

2017-04-07 Thread Kostia Balytskyi 
# HG changeset patch
# User Kostia Balytskyi 
# 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.

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')
+
 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@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel