Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Erick Mattos <erick.mattos@xxxxxxxxx>

> When we use one of the options above we are normally trying to do mainly
> two things: using the source as a template or recreating a commit with
> corrections.
>
> When they are used, the authorship and timestamp recorded in the newly
> created commit are always taken from the original commit.  And they
> should not when we are using it as a template or when our change to the
> code is so significant that we should take over the authorship (with the
> blame for bugs we introduce, of course).
>
> The new --reset-author option is meant to solve this need by
> regenerating the timestamp and setting as the new author the committer
> or the one specified by --author option.
>
> Signed-off-by: Erick Mattos <erick.mattos@xxxxxxxxx>
> ---

If you are sending an update to a previous patch (I am comparing
this patch with the "show by example" patch Junio sent on 11/02 
http://article.gmane.org/gmane.comp.version-control.git/131893), 
it is a common courtesy to summarize what you changed relative 
to the base version after the three dashes line, so that people 
will know which part can be skipped while reviewing your patch.

I was originally puzzled by Junio's "mine" because I thought 
"what does taking over the authorship have to do with digging 
diamonds out of earth?". "reset-author" makes it very clear,
and I think it is a much better name for the option.

Your first paragraph mentions "one of the options", but subject 
mentions four (-c, -C, --amend and --reset-author) and you want 
only the first three of them.

The second paragraph of gmane/131893 says "borrow the commit log 
message" instead of repeating "a template". It has the effect of 
clarifying that only the message part is borrowed but not the 
authorship information.

I think gmane/131893 has much better description than your first 
two paragraphs for the these reasons.

> diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
> new file mode 100755
> index 0000000..7290242
> --- /dev/null
> +++ b/t/t7509-commit.sh

I have to say that the test script is much worse than what 
gmane/131893 had.

The old test made sure that -C copied the message, with or 
without the --mine option. But this test only checks the 
author line (and it doesn't even make sure that "^author" 
matches only in the header). The messages are unchecked, 
and it will let a bug when someone breaks --reset-author 
logic in the future in such a way that it corrupts the 
message by mistake go unnoticed.

Also the old test was much more readable because it used 
shell functions to avoid repeating cat-file and pipe to sed 
script. It also tagged the initial commit, which had a nice 
effect that a failure in any intermediate test will not change 
which earlier commit is reused (eg. yours say "-C HEAD^" but 
old test said "-C Initial").

It looks silly to create an "Initial Commit" in the middle 
of history, too (^_^).

I think it is much better to replace "--mine" in gmane/131893 
with "--reset-author" and make no other change to the test.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]