Patchwork [7,of,7] verify: get rid of some unnecessary local variables

login
register
mail settings
Submitter Durham Goode
Date Jan. 6, 2016, 2:44 a.m.
Message ID <7764ec8ef96b89e432c5.1452048291@dev8486.prn1.facebook.com>
Download mbox | patch
Permalink /patch/12550/
State Accepted
Delegated to: Martin von Zweigbergk
Headers show

Comments

Durham Goode - Jan. 6, 2016, 2:44 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1452042494 28800
#      Tue Jan 05 17:08:14 2016 -0800
# Node ID 7764ec8ef96b89e432c5a7b2334abe8670ff4c08
# Parent  48638907e0b683734ca0e38502bcd0ce210a3590
verify: get rid of some unnecessary local variables

Now that all the major functionality has been refactored out, we can delete some
unused local variables.
Martin von Zweigbergk - Jan. 6, 2016, 5:19 a.m.
Looks good, although I've mostly reviewed the subject lines only (and tests
pass :-)) . Seems like a nice improvement to readability too. Pushed to the
clowncopter!
Martin von Zweigbergk - Jan. 6, 2016, 5:31 a.m.
On Tue, Jan 5, 2016 at 9:19 PM Martin von Zweigbergk <martinvonz@google.com>
wrote:

> Looks good, although I've mostly reviewed the subject lines only (and
> tests pass :-)) . Seems like a nice improvement to readability too. Pushed
> to the clowncopter!
>

After this series, _verifymanifest() and _verifychangelog() accept
some dictionaries as "output parameters". Do your extensions rely on that?
If not, I'll send a followup patch to make them return values.
Durham Goode - Jan. 6, 2016, 5:46 p.m.
On 1/5/16 9:31 PM, Martin von Zweigbergk wrote:
>
> On Tue, Jan 5, 2016 at 9:19 PM Martin von Zweigbergk 
> <martinvonz@google.com <mailto:martinvonz@google.com>> wrote:
>
>     Looks good, although I've mostly reviewed the subject lines only
>     (and tests pass :-)) . Seems like a nice improvement to
>     readability too. Pushed to the clowncopter!
>
>
> After this series, _verifymanifest() and _verifychangelog() accept 
> some dictionaries as "output parameters". Do your extensions rely on 
> that? If not, I'll send a followup patch to make them return values.
No, my extensions don't rely on that.  This was just a way of 
refactoring that had the least amount of code churn.
Martin von Zweigbergk - Jan. 6, 2016, 5:50 p.m.
On Wed, Jan 6, 2016 at 9:48 AM Durham Goode <durham@fb.com> wrote:

>
>
> On 1/5/16 9:31 PM, Martin von Zweigbergk wrote:
>
>
> On Tue, Jan 5, 2016 at 9:19 PM Martin von Zweigbergk <
> martinvonz@google.com> wrote:
>
>> Looks good, although I've mostly reviewed the subject lines only (and
>> tests pass :-)) . Seems like a nice improvement to readability too. Pushed
>> to the clowncopter!
>>
>
> After this series, _verifymanifest() and _verifychangelog() accept
> some dictionaries as "output parameters". Do your extensions rely on that?
> If not, I'll send a followup patch to make them return values.
>
> No, my extensions don't rely on that.  This was just a way of refactoring
> that had the least amount of code churn.
>

That's what I guessed. I'll send a followup patch once mpm has pushed these
patches to his repo (I still rely on pushgate :-().

Patch

diff --git a/mercurial/verify.py b/mercurial/verify.py
--- a/mercurial/verify.py
+++ b/mercurial/verify.py
@@ -137,10 +137,8 @@  class verifier(object):
         mflinkrevs = {}
         filelinkrevs = {}
         filenodes = {}
-        revisions = 0
-        badrevs = self.badrevs
+
         ui = repo.ui
-        cl = repo.changelog
 
         if not repo.url().startswith('file:'):
             raise error.Abort(_("cannot verify bundle or remote repos"))
@@ -148,10 +146,9 @@  class verifier(object):
         if os.path.exists(repo.sjoin("journal")):
             ui.warn(_("abandoned transaction found - run hg recover\n"))
 
-        revlogv1 = self.revlogv1
-        if ui.verbose or not revlogv1:
+        if ui.verbose or not self.revlogv1:
             ui.status(_("repository uses revlog format %d\n") %
-                           (revlogv1 and 1 or 0))
+                           (self.revlogv1 and 1 or 0))
 
         self._verifychangelog(mflinkrevs, filelinkrevs)
 
@@ -160,10 +157,9 @@  class verifier(object):
         self._crosscheckfiles(mflinkrevs, filelinkrevs, filenodes)
 
         totalfiles, filerevisions = self._verifyfiles(filenodes, filelinkrevs)
-        revisions += filerevisions
 
         ui.status(_("%d files, %d changesets, %d total revisions\n") %
-                       (totalfiles, len(cl), revisions))
+                       (totalfiles, len(repo.changelog), filerevisions))
         if self.warnings:
             ui.warn(_("%d warnings encountered!\n") % self.warnings)
         if self.fncachewarned:
@@ -171,9 +167,9 @@  class verifier(object):
                       'corrupt fncache\n'))
         if self.errors:
             ui.warn(_("%d integrity errors encountered!\n") % self.errors)
-            if badrevs:
+            if self.badrevs:
                 ui.warn(_("(first damaged changeset appears to be %d)\n")
-                        % min(badrevs))
+                        % min(self.badrevs))
             return 1
 
     def _verifychangelog(self, mflinkrevs, filelinkrevs):