Thread (4 messages) flat view 4 messages, 3 authors, 2016-06-15

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

From: Nanako Shiraishi <hidden>
Date: 2016-06-15 22:47:39

Quoting Erick Mattos [off-list ref]
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 <redacted>
---
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.
quoted hunk ↗ jump to hunk
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/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help