From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Beat Bolli <dev+git@drbeat.li>, Git List <git@vger.kernel.org>,
Paul Mackerras <paulus@samba.org>
Subject: Re: [PATCH v3] gitk: Add a "Copy commit summary" command
Date: Fri, 17 Jul 2015 10:28:07 -0700 [thread overview]
Message-ID: <xmqqwpxy1yoo.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPig+cR=u_ak_=J=gSAWfiNB01R7FBG+bCrx+k1HNAE0xHtwFQ@mail.gmail.com> (Eric Sunshine's message of "Fri, 17 Jul 2015 13:17:33 -0400")
Eric Sunshine <sunshine@sunshineco.com> writes:
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
>> Reviewed-by: Johannes Sixt <j6t@kdbg.org>
>
> You should drop these Reviewed-by: footers, as they imply that the
> code was thoroughly digested and the implementation deemed correct.
... and the most importantly, the named people said that themselves.
I do not think that happened here (yet).
>> +proc copysummary {} {
>> + global rowmenuid
>> +
>> + set format "%h (\"%s\", %ad)"
>> + set summary [exec git show -s --pretty=format:$format --date=short \
>> + $rowmenuid]
>> +
>> + clipboard clear
>> + clipboard append $summary
>> +}
>> +
I think this is a reasonable implementation. The usual "spawning a
process for each commit is too expensive" would not apply, because
it is done on demand only for the single commit that the end-user
specified.
next prev parent reply other threads:[~2015-07-17 17:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-17 8:39 [PATCH v3] gitk: Add a "Copy commit summary" command Beat Bolli
2015-07-17 17:17 ` Eric Sunshine
2015-07-17 17:28 ` Junio C Hamano [this message]
2015-07-17 20:59 ` Beat Bolli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwpxy1yoo.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=dev+git@drbeat.li \
--cc=git@vger.kernel.org \
--cc=paulus@samba.org \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.