Erick Mattos <erick.mattos@xxxxxxxxx> writes:
> Cutting --author away would make impossible for someone to force a new author
> with a new timestamp in case he is templating. As an example he can be using
> the --author because he is doing a change in a computer not his own or
> something alike.
Sorry, but I cannot help feeling a bit frustrated and mildly irritated.
I had an impression that we have already established that setting the
author with --author="Somebody Else <s@xxx>" and committing with the
current time does not make much sense from the workflow point of view long
time ago in this thread.
The mail transport might have mangled the name, and when using --amend (or
read-tree followed by commit -c), it is handy to fix the mangled name by
using --author, but in such a case you would actively want to keep the
timestamp obtained from the e-mail via either --amend or -c.
But allowing this combination, even though it might not make much sense,
is just giving extra length to the rope, so it may not be such a big deal.
I didn't feel motivated enough to read the whole thing while other patches
are in my inbox, so I instead ran diff between the previous one (without
my suggestion today) and this round.
I see that you fixed a lot of grammar in the log message of my earlier
suggestion, all of which looked very good. Also you added a check in the
program to make sure that --renew is given only when -C/-c/--amend is
given, which is also good. Neither of our set of tests checks this
condition, though. IOW, we would need to add something like this at the
end of my version (adjust to --reset-author for your version):
test_expect_success '--mine should be rejected without -c/-C/--amend' '
git checkout Initial &&
echo "Test 7" >>foo &&
test_tick &&
test_must_fail git commit -a --mine -m done
'
I am not sure why you insist to use your version of test script and keep
changing it, though. It looks a lot worse even only after reviewing its
early part.
- author_id runs an extra grep that is unnecessary. The separation of
_id and _timestamp are unnecessary if you checked against an expected
author ident and timestamp as a single string, i.e.
FRIGATE='Frigate <flying@xxxxxxxxxx>' ;# do this only once at the beginning
...
git commit -C HEAD --reset-author --author="$FRIGATE" &&
echo "author $FRIGATE $GIT_AUTHOR_TIME" >expect &&
author_header HEAD >actual &&
test_cmp expect actual
This becomes irrelevant if we don't support mixing --renew and
--author, of course.
- message_body() now has a backslash whose sole purpose is to be an
eyesore.
- initiate_test() does not string the commands together with &&
I might change my mind after I take a break, review others' patches, and
spend some time on my own hacking on other topics before revisiting this
patch, but at this point I find that reviewing newer rounds of this series
has rather quickly diminishing value, and more time is being spent on
teaching shell scripting to you rather than on polishing the end result.
Sorry, but I cannot help feeling a bit frustrated and mildly irritated.
Time to take a break and attend other topics for a change.
--
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