Patchwork [2,of,4,shelve-ext,v3] shelve: move node-cleaning functionality to be a member of shelvedstate

login
register
mail settings
Submitter Kostia Balytskyi
Date April 10, 2017, 11:28 p.m.
Message ID <1f3af1e9d43286254237.1491866914@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/20098/
State Accepted
Headers show

Comments

Kostia Balytskyi - April 10, 2017, 11:28 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1491864494 25200
#      Mon Apr 10 15:48:14 2017 -0700
# Node ID 1f3af1e9d43286254237417adf26ecc97efa62e6
# Parent  18d5dc2e81ff328facddd7c0294d7301a057504d
shelve: move node-cleaning functionality to be a 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 valuable in future, when a better
hiding mechanism is introduced.
Yuya Nishihara - April 12, 2017, 2:45 p.m.
On Mon, 10 Apr 2017 16:28:34 -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1491864494 25200
> #      Mon Apr 10 15:48:14 2017 -0700
> # Node ID 1f3af1e9d43286254237417adf26ecc97efa62e6
> # Parent  18d5dc2e81ff328facddd7c0294d7301a057504d
> shelve: move node-cleaning functionality to be a 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 valuable in future, when a 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 removenodes(self, ui, repo):
> +        """Cleanup temporary nodes from the repo"""
> +        repair.strip(ui, repo, self.nodestoremove, backup=False,
> +                     topic='shelve')

This looked good at first, but the last patch makes me feel this might be
bad for storage abstraction. See the comment on the patch 4 for details.

Patch

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 removenodes(self, ui, repo):
+        """Cleanup temporary nodes from the repo"""
+        repair.strip(ui, repo, self.nodestoremove, 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.nodestoremove, backup=False,
-                         topic='shelve')
+            state.removenodes(ui, repo)
         finally:
             shelvedstate.clear(repo)
             ui.warn(_("unshelve of '%s' aborted\n") % state.name)
@@ -655,8 +659,7 @@  def unshelvecontinue(ui, repo, state, op
         mergefiles(ui, repo, state.wctx, shelvectx)
         restorebranch(ui, repo, state.branchtorestore)
 
-        repair.strip(ui, repo, state.nodestoremove, backup=False,
-                     topic='shelve')
+        state.removenodes(ui, repo)
         _restoreactivebookmark(repo, state.activebookmark)
         shelvedstate.clear(repo)
         unshelvecleanup(ui, repo, state.name, opts)