Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

From: Erick Mattos <erick.mattos@gmail.com>
Date: Oct 30, 2009, 22:13 UTC
Message-ID: <55bacdd30910301513u6ba6a575w2c65358ff368aeab@mail.gmail.com>
In-Reply-To: <55bacdd30910301505xe712b74m837dc862a6ee953@mail.gmail.com>

2009/10/30 Junio C Hamano <gitster@pobox.com>:

> Erick Mattos <erick.mattos@gmail.com> writes:
>
> A patch always changes something so the title "Changed ... behavior" does
> not carry enough information

Sorry but I thought It was enough.  First submitted patch.

>(besides, you write logs as if you are making
> an order to the codebase to "do this!").

Not a chance! Just trying to help. A way for paying back all the benefits I enjoy by your software.

Show 12 quoted lines

>> The code herein changes commit timestamp recording from a source in a
>> more intuitive way.
>>
>> Description:
>
> Remove the above.  Instead, start with a description of what the current
> code does, e.g.
>
>    Subject: commit -c/-C/--amend: allow 'current' timestamp to be used
>
>    When these options are used, the timestamp recorded in the newly
>    created commit is always taken from the original commit.

Demand accepted.

Show 19 quoted lines

>
> Then the rest of your text flows much more nicely...
>
>> When we use one of the options above we are normally trying to do mainly
>> two things: one is using the source as a template and second is to
>> recreate a commit with corrections.
>>
>> In the first case timestamp should by default be taken by the time we
>> are doing the commit, not by the source.  On the second case the actual
>> behavior is acceptable.
>
> ... and the reader does not have to wonder what "the actual behavior" is;
> instead you can say "the current behavior" here.
>
>> ...
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Also the reader does not have to wonder what "the same" means here.

Done again.

Show 10 quoted lines

> I agree that the issue the patch addresses is worth improving, and I think
> it is sensible to default to reuse the timestamp for -C and not to reuse
> for --amend.  I am not sure about -c myself, but it probably shouldn't
> reuse the timestamp by default.
>
> I however think (old|new)-timestamp is suboptimal.
>
> We already have --reuse-message, so why not trigger this with a single
> option --(no-)reuse-timestamp?
>

Don't you think it would be a little big? I had compared the option name so it would be more or less of reuse-message.