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

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

 



Erick Mattos <erick.mattos@xxxxxxxxx> writes:

A patch always changes something so the title "Changed ... behavior" does
not carry enough information (besides, you write logs as if you are making
an order to the codebase to "do this!").

> 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.

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.

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?
--
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]