git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Mehul Jain <mehul.jain2029@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v3 0/3] Introduce log.showSignature config variable
Date: Thu, 23 Jun 2016 11:14:27 +0530	[thread overview]
Message-ID: <CA+DCAeQUJ4D4kjR6FtTibwPO8o+wv-hQNDDj2ysJaBHGTPkHgA@mail.gmail.com> (raw)
In-Reply-To: <xmqqr3bpklsd.fsf@gitster.mtv.corp.google.com>

On Thu, Jun 23, 2016 at 2:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Mehul Jain <mehul.jain2029@gmail.com> writes:
>
>> Add a new configuratation variable "log.showSignature" for git-log
>> and related commands. "log.showSignature=true" will enable user to
>> see GPG signature by default for git-log and related commands.
>>
>> Changes compared to v2:
>>       * A preparatory patch 1/3 has been introduced so that tests
>>         in patches 2/3 and 3/3 can take advantage of it.
>
> It is unclear how this change allows the remainder to "take
> advanrage" to me.  Earlier, "signed" branch was created only when
> the GPG prerequisite is met and with this change the branch is
> always created, which is the only change as far as I can see.  But
> the tests that are added in 2 and 3 are all protected with the GPG
> prerequiste.
>
> Besides, the invocation of "git commit -S" after this change is no
> longer protected by the GPG prerequisite and it may even cause the
> 'setup' step to fail on a host without GPG.

I overlooked the GPG prerequisite when I created the "setup signed
branch" test in patch 1/3. I will send a patch to  rectify it ones
everyone agree with the idea behind this patch.

In patch 2/3 and 3/3, there are many tests which requires a branch
similar to that of "signed" branch, i.e. a branch with a commit having
GPG signature. So previously in v2, I created two new branches,
"test_sign" and "no_sign", which are identical to that of "signed"
branch. And with these branches, I wrote the tests in patch 2/3
and 3/3.

As suggested by Eric [1], rather than creating new branches, I
can take advantage of "signed" branch which already exists.
So, I created a new test to separate the creation of "signed" branch
from existing test "log --graph --show-signature". This was done
because I do not want new tests to depend on this test. If in future
someone changes this test then it will affect new tests introduced
in 2/3 and 3/3.

Now the new tests and existing one ("log --graph ... ") are using a
single branch "signed" to do there work.

If changing an existing test is not well justified here, then I can create
setup test for new tests only, without affecting the existing test.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/297648

Thanks,
Mehul

  reply	other threads:[~2016-06-23  5:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-22 16:51 [PATCH v3 0/3] Introduce log.showSignature config variable Mehul Jain
2016-06-22 16:51 ` [PATCH v3 1/3] t4202: refactor test Mehul Jain
2016-06-24 14:12   ` Mehul Jain
2016-06-22 16:51 ` [PATCH v3 2/3] log: add "--no-show-signature" command line option Mehul Jain
2016-06-22 16:51 ` [PATCH v3 3/3] log: add log.showSignature configuration variable Mehul Jain
2016-06-22 20:31 ` [PATCH v3 0/3] Introduce log.showSignature config variable Junio C Hamano
2016-06-23  5:44   ` Mehul Jain [this message]
2016-06-23  6:32     ` Junio C Hamano
2016-06-24  9:21       ` Mehul Jain
2016-06-24 12:25         ` Eric Sunshine

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+DCAeQUJ4D4kjR6FtTibwPO8o+wv-hQNDDj2ysJaBHGTPkHgA@mail.gmail.com \
    --to=mehul.jain2029@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).