git map-branches: add --show-subject

git map-branches: add --show-subject

BUG=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=294578

[borenet, 2015-03-10 15:18:57]

borenet@google.com changed reviewers:
+ agable@chromium.org, calamity@chromium.org

[iannucci, 2015-03-10 18:42:19]

iannucci@chromium.org changed reviewers:
+ iannucci@chromium.org

[iannucci, 2015-03-10 18:42:19]

lgtm, though I wonder if this should just go under -vvv instead of growing a new
option. If someone complains about it suddenly show up when they ask for 'hella
verbose' mode, then we can turn it into a new option (or start thinking about
having a .gitconfig section for map-branches)

[borenet, 2015-03-10 18:44:43]

On 2015/03/10 18:42:19, iannucci wrote:
> lgtm, though I wonder if this should just go under -vvv instead of growing a
new
> option. If someone complains about it suddenly show up when they ask for
'hella
> verbose' mode, then we can turn it into a new option (or start thinking about
> having a .gitconfig section for map-branches)

That's okay with me too.  I did it this way because with the codereview URL as
well as the commit subject you end up with some wrapping which makes things not
so readable.

[iannucci, 2015-03-10 18:52:41]

On 2015/03/10 at 18:44:43, borenet wrote:
> On 2015/03/10 18:42:19, iannucci wrote:
> > lgtm, though I wonder if this should just go under -vvv instead of growing a
new
> > option. If someone complains about it suddenly show up when they ask for
'hella
> > verbose' mode, then we can turn it into a new option (or start thinking
about
> > having a .gitconfig section for map-branches)
> 
> That's okay with me too.  I did it this way because with the codereview URL as
well as the commit subject you end up with some wrapping which makes things not
so readable.

TBH, I would consider shortening the url to crrev.com/r/<issue num>

let me go make that a real endpoint on crrev.com :D brb

[agable, 2015-03-10 22:14:06]

On 2015/03/10 at 18:52:41, iannucci wrote:
> On 2015/03/10 at 18:44:43, borenet wrote:
> > On 2015/03/10 18:42:19, iannucci wrote:
> > > lgtm, though I wonder if this should just go under -vvv instead of growing
a new
> > > option. If someone complains about it suddenly show up when they ask for
'hella
> > > verbose' mode, then we can turn it into a new option (or start thinking
about
> > > having a .gitconfig section for map-branches)
> > 
> > That's okay with me too.  I did it this way because with the codereview URL
as well as the commit subject you end up with some wrapping which makes things
not so readable.
> 
> TBH, I would consider shortening the url to crrev.com/r/<issue num>
> 
> let me go make that a real endpoint on crrev.com :D brb

crrev.com/<issue num> already works, even without the /r/, because issue numbers
are a few orders of magnitude larger than svn numbers.

I would also likely put this under verbose mode rather than growing another
flag, but am happy either way.

[iannucci, 2015-03-10 22:27:18]

Yeah, but... the crrev.com/<magic!!> redirection scares me a lot :(

On Tue, Mar 10, 2015 at 3:14 PM <agable@chromium.org> wrote:

> On 2015/03/10 at 18:52:41, iannucci wrote:
> > On 2015/03/10 at 18:44:43, borenet wrote:
> > > On 2015/03/10 18:42:19, iannucci wrote:
> > > > lgtm, though I wonder if this should just go under -vvv instead of
> > growing
> a new
> > > > option. If someone complains about it suddenly show up when they ask
> > for
> 'hella
> > > > verbose' mode, then we can turn it into a new option (or start
> > thinking
> about
> > > > having a .gitconfig section for map-branches)
> > >
> > > That's okay with me too.  I did it this way because with the codereview
> > URL
> as well as the commit subject you end up with some wrapping which makes
> things
> not so readable.
>
> > TBH, I would consider shortening the url to crrev.com/r/<issue num>
>
> > let me go make that a real endpoint on crrev.com :D brb
>
> crrev.com/<issue num> already works, even without the /r/, because issue
> numbers
> are a few orders of magnitude larger than svn numbers.
>
> I would also likely put this under verbose mode rather than growing another
> flag, but am happy either way.
>
> https://codereview.chromium.org/996643004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[calamity, 2015-03-12 21:30:48]

lgtm.

I'm also for having a separate flag due to the wrapping issue. Also, please note
I've landed a CL that makes -vvv grab the CL status from the server and color
the CLs correctly. So adding this under another v means we have to either always
look at the remote status (which is slow) to look at the subjects, or deal with
wrapping issues when looking at the remote CL status.

Somebody's gonna need to grow a new flag methinks.

[iannucci, 2015-03-12 21:37:50]

On 2015/03/12 at 21:30:48, calamity wrote:
> lgtm.
> 
> I'm also for having a separate flag due to the wrapping issue. Also, please
note I've landed a CL that makes -vvv grab the CL status from the server and
color the CLs correctly. So adding this under another v means we have to either
always look at the remote status (which is slow) to look at the subjects, or
deal with wrapping issues when looking at the remote CL status.
> 
> Somebody's gonna need to grow a new flag methinks.

blarrrrrrg more flags :(

what do people think about having a map-branches.ui gitconfig section? it's
still (sort of) flags, but then you can add+forget them. The manpage for
map-branches probably needs to be updated at this point anyway, too.

[borenet, 2015-03-25 15:48:08]

Friendly ping.  What's the best way forward with this.  Should I commit as-is or
should we come up with a nicer way to express the options through flags or
config file?

[iannucci, 2015-03-25 16:17:28]

On 2015/03/25 15:48:08, borenet wrote:
> Friendly ping.  What's the best way forward with this.  Should I commit as-is
or
> should we come up with a nicer way to express the options through flags or
> config file?

I'm still thinking that a config section is the way to go, but for this CL a new
flag is probably the way to go.

[iannucci, 2015-03-25 16:20:29]

On 2015/03/25 16:17:28, iannucci wrote:
> On 2015/03/25 15:48:08, borenet wrote:
> > Friendly ping.  What's the best way forward with this.  Should I commit
as-is
> or
> > should we come up with a nicer way to express the options through flags or
> > config file?
> 
> I'm still thinking that a config section is the way to go, but for this CL a
new
> flag is probably the way to go.

(Too many ways to go in there...)

[borenet, 2015-03-26 13:41:50]

On 2015/03/25 16:20:29, iannucci wrote:
> On 2015/03/25 16:17:28, iannucci wrote:
> > On 2015/03/25 15:48:08, borenet wrote:
> > > Friendly ping.  What's the best way forward with this.  Should I commit
> as-is
> > or
> > > should we come up with a nicer way to express the options through flags or
> > > config file?
> > 
> > I'm still thinking that a config section is the way to go, but for this CL a
> new
> > flag is probably the way to go.
> 
> (Too many ways to go in there...)

Okay. I'll submit as-is and let you guys figure out how you want to reorganize
things later.

[borenet, 2015-03-26 13:41:53]

The CQ bit was checked by borenet@google.com

[commit-bot: I haz the power, 2015-03-26 13:42:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/996643004/1

[commit-bot: I haz the power, 2015-03-26 13:42:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-26 13:42:10]

Failed to apply patch for depot_tools/git_map_branches.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file depot_tools/git_map_branches.py
  Hunk #2 FAILED at 110.
  Hunk #3 succeeded at 259 with fuzz 2 (offset 14 lines).
  Hunk #4 FAILED at 270.
  2 out of 4 hunks FAILED -- saving rejects to file
depot_tools/git_map_branches.py.rej

Patch:       depot_tools/git_map_branches.py
Index: git_map_branches.py
diff --git depot_tools/git_map_branches.py depot_tools/git_map_branches.py
index
0bf57499e738d0781da76551f75363dbe46d1fed..e4fc80b6717b4d337672b093a8ee7e7221b158a1
100755
--- a/depot_tools/git_map_branches.py
+++ b/depot_tools/git_map_branches.py
@@ -35,6 +35,7 @@ from third_party.colorama import Fore, Style
 
 from git_common import current_branch, upstream, tags, get_branches_info
 from git_common import get_git_version, MIN_UPSTREAM_TRACK_GIT_VERSION,
hash_one
+from git_common import run
 
 DEFAULT_SEPARATOR = ' ' * 4
 
@@ -109,6 +110,7 @@ class BranchMapper(object):
 
   def __init__(self):
     self.verbosity = 0
+    self.show_subject = False
     self.output = OutputManager()
     self.__gone_branches = set()
     self.__branches_info = None
@@ -244,6 +246,10 @@ class BranchMapper(object):
           branchref=branch).GetIssueURL() if branch_hash else None
       line.append(url or none_text, color=Fore.BLUE if url else Fore.WHITE)
 
+    # The subject of the most recent commit on the branch.
+    if self.show_subject:
+      line.append(run('log', '-n1', '--format=%s', branch))
+
     self.output.append(line)
 
     for child in sorted(self.__parent_map.pop(branch, ())):
@@ -265,12 +271,15 @@ def main(argv):
                       help='Display branch hash and Rietveld URL')
   parser.add_argument('--no-color', action='store_true', dest='nocolor',
                       help='Turn off colors.')
+  parser.add_argument('--show-subject', action='store_true',
+                      dest='show_subject', help='Show the commit subject.')
 
   opts = parser.parse_args(argv)
 
   mapper = BranchMapper()
   mapper.verbosity = opts.v
   mapper.output.nocolor = opts.nocolor
+  mapper.show_subject = opts.show_subject
   mapper.start()
   print mapper.output.as_formatted_string()
   return 0

[borenet, 2015-03-26 13:46:11]

The CQ bit was checked by borenet@google.com

[borenet, 2015-03-26 13:46:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
calamity@chromium.org, iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/996643004/#ps20001 (title:
"rebase")

[commit-bot: I haz the power, 2015-03-26 13:46:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/996643004/20001

[commit-bot: I haz the power, 2015-03-26 13:50:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-26 13:50:18]

Presubmit check for 996643004-20001 failed and returned exit status 1.

Running presubmit commit checks ...
Checking out rietveld...
Running save-description-on-failure.sh
Running push-basic.sh
Running upstream.sh
Running submit-from-new-dir.sh
Running abandon.sh
Running submodule-merge-test.sh
Running upload-local-tracking-branch.sh
Running hooks.sh
Running post-dcommit-hook-test.sh
Running upload-stale.sh
Running patch.sh
Running basic.sh

** Presubmit ERRORS **
tests/gclient_smoketest.py (64.31s) failed
Ffatal: reference is not a tree: b92a7755827b77ba23f6e4640e362b4b95b4d5a5
EE..............................................
======================================================================
ERROR: testBlinkDEPSChangeUsingGit (__main__.BlinkDEPSTransitionSmokeTest)
Like testBlinkDEPSChangeUsingGclient, but move the main project using
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1630, in testBlinkDEPSChangeUsingGit
    cwd=self.checkout_path)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subprocess2.py", line
484, in check_call
    check_call_out(args, **kwargs)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subprocess2.py", line
478, in check_call_out
    returncode, args, kwargs.get('cwd'), out[0], out[1])
CalledProcessError: Command git checkout -q
b92a7755827b77ba23f6e4640e362b4b95b4d5a5 returned non-zero exit status 128 in
/tmp/trialk6ev8s/__main__.BlinkDEPSTransitionSmokeTest.testBlinkDEPSChangeUsingGit/src

======================================================================
ERROR: testBlinkLocalBranchesArePreserved
(__main__.BlinkDEPSTransitionSmokeTest)
Checks that the state of local git branches are effectively preserved
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1656, in
testBlinkLocalBranchesArePreserved
    self.CheckStatusPreMergePoint()
  File "tests/gclient_smoketest.py", line 1558, in CheckStatusPreMergePoint
    self.blink), self.blink_git_url)
  File "/b/infra_internal/commit_queue/workdir/tools/depot_tools/scm.py", line
121, in Capture
    cwd=cwd, stderr=subprocess2.PIPE, env=env, **kwargs)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subprocess2.py", line
515, in check_output
    return check_call_out(args, stdout=PIPE, **kwargs)[0]
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subprocess2.py", line
475, in check_call_out
    out, returncode = communicate(args, **kwargs)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subprocess2.py", line
449, in communicate
    proc = Popen(args, **kwargs)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/subprocess2.py", line
253, in __init__
    % (str(e), kwargs.get('cwd'), args[0]))
OSError: Execution failed with error: [Errno 2] No such file or directory:
'/tmp/trialk6ev8s/__main__.BlinkDEPSTransitionSmokeTest.testBlinkLocalBranchesArePreserved/src/third_party/WebKit'.
Check that
/tmp/trialk6ev8s/__main__.BlinkDEPSTransitionSmokeTest.testBlinkLocalBranchesArePreserved/src/third_party/WebKit
or git exist and have execution permission.

======================================================================
FAIL: testBlinkDEPSChangeUsingGclient (__main__.BlinkDEPSTransitionSmokeTest)
Checks that {src,blink} repos are consistent when syncing going back and
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 1601, in
testBlinkDEPSChangeUsingGclient
    self.assertEqual(res[2], 0, 'DEPS change sync failed.')
AssertionError: DEPS change sync failed.

----------------------------------------------------------------------
Ran 49 tests in 64.145s

FAILED (failures=1, errors=2)


Presubmit checks took 194.6s to calculate.

[borenet, 2015-03-26 14:06:50]

The CQ bit was checked by borenet@google.com

[commit-bot: I haz the power, 2015-03-26 14:06:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/996643004/20001

[commit-bot: I haz the power, 2015-03-26 14:10:09]

Committed patchset #2 (id:20001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=294578