Patchwork [stable] histedit: use str.startswith, remove duplicate line and extra blank line

login
register
mail settings
Submitter adgar@google.com
Date Aug. 7, 2014, 7:17 p.m.
Message ID <4fc87e9ba34429406ff2.1407439048@adgar.nyc.corp.google.com>
Download mbox | patch
Permalink /patch/5316/
State Changes Requested
Headers show

Comments

adgar@google.com - Aug. 7, 2014, 7:17 p.m.
# HG changeset patch
# User Mike Edgar <adgar@google.com>
# Date 1407427942 14400
#      Thu Aug 07 12:12:22 2014 -0400
# Node ID 4fc87e9ba34429406ff29446c5fdd8b5a17687e1
# Parent  ba5fc3f81f1588507a5a84ac2b43679705ceec7b
histedit: use str.startswith, remove duplicate line and extra blank line
Pierre-Yves David - Aug. 7, 2014, 7:19 p.m.
On 08/07/2014 12:17 PM, adgar@google.com wrote:
> # HG changeset patch
> # User Mike Edgar <adgar@google.com>
> # Date 1407427942 14400
> #      Thu Aug 07 12:12:22 2014 -0400
> # Node ID 4fc87e9ba34429406ff29446c5fdd8b5a17687e1
> # Parent  ba5fc3f81f1588507a5a84ac2b43679705ceec7b
> histedit: use str.startswith, remove duplicate line and extra blank line

Looks like there is three different clean up in this patch. Can you 
split it?
Matt Mackall - Aug. 7, 2014, 8:01 p.m.
On Thu, 2014-08-07 at 15:17 -0400, adgar@google.com wrote:
> # HG changeset patch
> # User Mike Edgar <adgar@google.com>
> # Date 1407427942 14400
> #      Thu Aug 07 12:12:22 2014 -0400
> # Node ID 4fc87e9ba34429406ff29446c5fdd8b5a17687e1
> # Parent  ba5fc3f81f1588507a5a84ac2b43679705ceec7b
> histedit: use str.startswith, remove duplicate line and extra blank line

FYI, you've flagged this for stable, but the goal of stable is to
maximize the improvement/risk ratio. "Code cleanups" like this can only
negatively impact that ratio.. by accidentally introducing breakage.

http://mercurial.selenic.com/wiki/TimeBasedReleasePlan#Rules_for_code_freeze_and_stable_branch_commits

If you're going to tidy up histedit, it's best to group all your related
changes:

- whitespace changes (can all be in one patch)
- pure code movement
- typo fixes
- etc..

If I can look at each hunk of your patch and say "yes, that obviously
matches the one line description of this patch", then that's a good sign
you're doing it right.

(You're going to discover we really sweat the small stuff like this - it
reduces friction later when we get to the big stuff.)

I actually just took a moment to do:

$ emacs $(hg locate 'set:**.py and grep("(?m)\n\n\n\n")')

..to globally nuke some excessive vertical whitespace.

Patch

diff -r ba5fc3f81f15 -r 4fc87e9ba344 hgext/histedit.py
--- a/hgext/histedit.py	Tue Jun 17 20:26:51 2014 -0700
+++ b/hgext/histedit.py	Thu Aug 07 12:12:22 2014 -0400
@@ -209,7 +209,6 @@ 
     return commitfunc
 
 
-
 def applychanges(ui, repo, ctx, opts):
     """Merge changeset from ctx (only) in the current working directory"""
     wcpar = repo.dirstate.parents()[0]
@@ -602,11 +601,10 @@ 
             rules = f.read()
             f.close()
         rules = [l for l in (r.strip() for r in rules.splitlines())
-                 if l and not l[0] == '#']
+                 if not l.startswith('#')]
         rules = verifyrules(rules, repo, ctxs)
 
         parentctx = repo[root].parents()[0]
-        keep = opts.get('keep', False)
         replacements = []