git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] request-pull: avoid mentioning that the start point is a single commit
@ 2010-01-29  1:18 Miklos Vajna
  2010-01-29  1:19 ` Shawn O. Pearce
  2010-01-29  7:33 ` Junio C Hamano
  0 siblings, 2 replies; 4+ messages in thread
From: Miklos Vajna @ 2010-01-29  1:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Previously we ran shortlog on the start commit which always printed
"(1)" after the start commit, which gives no information, but makes the
output less easy to read. Avoid doing so.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

So for example the 'git request-pull master~2 . master' output diff is
the following here:

	 The following changes since commit 68186857a9bb0a71e9456155623e02d398a5b817:
	-  Junio C Hamano (1):
	-        Merge branch 'il/maint-colon-address'
	+  Junio C Hamano: Merge branch 'il/maint-colon-address'

	 are available in the git repository at:

 git-request-pull.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 630cedd..8475919 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -66,7 +66,7 @@ if [ -z "$branch" ]; then
 fi
 
 echo "The following changes since commit $baserev:"
-git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/  \1/'
+git log --max-count=1 --pretty=format:"  %an: %s%n%n" $baserev
 
 echo "are available in the git repository at:"
 echo
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] request-pull: avoid mentioning that the start point is a single commit
  2010-01-29  1:18 [PATCH] request-pull: avoid mentioning that the start point is a single commit Miklos Vajna
@ 2010-01-29  1:19 ` Shawn O. Pearce
  2010-01-29  7:33 ` Junio C Hamano
  1 sibling, 0 replies; 4+ messages in thread
From: Shawn O. Pearce @ 2010-01-29  1:19 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git

Miklos Vajna <vmiklos@frugalware.org> wrote:
> Previously we ran shortlog on the start commit which always printed
> "(1)" after the start commit, which gives no information, but makes the
> output less easy to read. Avoid doing so.
> 
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---

Acked-by: Shawn O. Pearce <spearce@spearce.org>

Nice fix, that was annoying me too, but I didn't care enough to
write a patch.  :-)
 

> -git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/  \1/'
> +git log --max-count=1 --pretty=format:"  %an: %s%n%n" $baserev

-- 
Shawn.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] request-pull: avoid mentioning that the start point is a single commit
  2010-01-29  1:18 [PATCH] request-pull: avoid mentioning that the start point is a single commit Miklos Vajna
  2010-01-29  1:19 ` Shawn O. Pearce
@ 2010-01-29  7:33 ` Junio C Hamano
  2010-01-29 14:17   ` [PATCH v2] " Miklos Vajna
  1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2010-01-29  7:33 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git

Miklos Vajna <vmiklos@frugalware.org> writes:

> Previously we ran shortlog on the start commit which always printed
> "(1)" after the start commit, which gives no information, but makes the
> output less easy to read. Avoid doing so.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> So for example the 'git request-pull master~2 . master' output diff is
> the following here:
>
> 	 The following changes since commit 68186857a9bb0a71e9456155623e02d398a5b817:
> 	-  Junio C Hamano (1):
> 	-        Merge branch 'il/maint-colon-address'
> 	+  Junio C Hamano: Merge branch 'il/maint-colon-address'
>
> 	 are available in the git repository at:
>
>  git-request-pull.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index 630cedd..8475919 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -66,7 +66,7 @@ if [ -z "$branch" ]; then
>  fi
>  
>  echo "The following changes since commit $baserev:"
> -git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/  \1/'
> +git log --max-count=1 --pretty=format:"  %an: %s%n%n" $baserev

A few comments:

 - Modernising implementation by using tools different from what the
   original used (i.e. shortlog -> log) is fine, but I'd recommend doing
   so even more thoroughly.  Use "show -s" instead of "log -1" and
   "--format=" instead of "--pretty=format:", for example.

 - If the stated goal of the change is to remove " (1)" which is
   distracting with no useful information, remove that and only that,
   without changing anything else in the output.

 - On the other hand, if you find that "AuthorName: " part is less useful
   in identifying the commit than its title to help the requestee, change
   the whole thing to make it even more useful.

So I'd suggest either:

	git show -s --format="  %an:%n        %s" $baserev

to be conservative, or

	git show -s --format="  %s (%an)" $baserev

or even to this:

	git show -s --format="  %s (%ci)" $baserev

I suspect that the last one would be the easiest for the requestee to
judge the freshness of the branch.

Why isn't the "The following changes..." line not part of the --format
thing, by the way?  From the POV of readability of the code (not
necessarily the output), doing it this way might be the cleanest:

-- >8 --
git show -s --format='The following changes since %H

    %s (%ci)

are available in the git repository at:' $baserev
echo "  $url $branch"
-- 8< --

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] request-pull: avoid mentioning that the start point is a single commit
  2010-01-29  7:33 ` Junio C Hamano
@ 2010-01-29 14:17   ` Miklos Vajna
  0 siblings, 0 replies; 4+ messages in thread
From: Miklos Vajna @ 2010-01-29 14:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Previously we ran shortlog on the start commit which always printed
"(1)" after the start commit, which gives no information, but makes the
output less easy to read. Avoid doing so.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Thu, Jan 28, 2010 at 11:33:13PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I suspect that the last one would be the easiest for the requestee to
> judge the freshness of the branch.
>
> Why isn't the "The following changes..." line not part of the --format
> thing, by the way?

OK, I changed it accordingly, though I did not the change the output of
the "commit <hash>" line. So the output diff is:

         The following changes since commit 103209c6782586d92b04ee1fc71c0fd6f6385f5f:
        -  Junio C Hamano (1):
        -        Merge branch 'jc/maint-reflog-bad-timestamp'
        +
        +  Merge branch 'jc/maint-reflog-bad-timestamp' (2010-01-27 14:57:37 -0800)

         are available in the git repository at:

 git-request-pull.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index 630cedd..8fd15f6 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -65,11 +65,11 @@ if [ -z "$branch" ]; then
 	status=1
 fi
 
-echo "The following changes since commit $baserev:"
-git shortlog --max-count=1 $baserev | sed -e 's/^\(.\)/  \1/'
+git show -s --format='The following changes since commit %H:
 
-echo "are available in the git repository at:"
-echo
+  %s (%ci)
+
+are available in the git repository at:' $baserev
 echo "  $url $branch"
 echo
 
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-01-29 14:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-29  1:18 [PATCH] request-pull: avoid mentioning that the start point is a single commit Miklos Vajna
2010-01-29  1:19 ` Shawn O. Pearce
2010-01-29  7:33 ` Junio C Hamano
2010-01-29 14:17   ` [PATCH v2] " Miklos Vajna

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).