Thiago Farina <tfransosi@xxxxxxxxx> writes:
> They follow the below pattern:
> Git command: git command-name
> Usage string: git_command_name_usage
>
> Also changes "static char const * const" to "static const char * const" to
> match with the definition in api-parse-options.txt.
Probably it is obvious to you but may I ask a very stupid question?
People have to spend time to review 2500 lines of changes to make sure
that this patch does not contain silly mistakes, unexpected side effects
nor malicious changes.
And after spending that time, we need to suffer merge conflicts when we
merge a topic for this patch to 'next' (or 'pu'), because there are topics
that update nearby lines.
Then it gets worse. When somebody needs to make a real change (as opposed
to a cosmetic one) to a topic that is cooking in 'next' that affects lines
near this patch touches, there are two choices, neither of which is good:
(1) make a patch against 'next', in which case I have to reverse rebase
the patch to keep the old variable name and formatting, as this "Trivial
fix" is not part of what the topic is about; or (2) make a patch against
the tip of the topic branch, in which case I have to resolve conflicts
when the updated topic is merged to 'next'.
In addition to your time spent to produce this patch and a couple of its
earlier iterations, all of the above is the cost of this whole thing.
Now that I talked about the cost, I have to ask about benefit.
What does this _fix_? Does the benefit of this patch outweigh the cost,
and if it does, in what way?
In earlier iterations I couldn't answer this question myself, and I still
cannot. That is the kind of patch people call needless code churn.
> -static char const * const archive_usage[] = {
> +static const char * const git_archive_usage[] = {
> -static const char * const builtin_add_usage[] = {
> +static const char * const git_add_usage[] = {
> ...
> -static const char * const git_bisect_helper_usage[] = {
> +static const char * const builtin_bisect_helper_usage[] = {
Huh? This doesn't match what you claimed in the log message.
I see v2 and v3 were posted 15 minutes apart. People need to check both
to see what changed, and that also adds to the cost.
Please slow down.
I do not mean "please slow down working on your patch, and instead spend
time somewhere else". I mean "please spend more time rre-re-re-reviewing
what you are going to send." If you did so, externally it would look as
if you slowed down ;-).
Among the topics that are cooking in 'next', at least builtin-commit.c and
fast-import.c have changes to overlapping areas this huge patch touches.
At least, exclude the changes to them from this patch, and make patches
separately for them. That would make the cost of conflict resolution
slightly smaller. You need to also check with 'pu', but writing this
response has already made me exceed my git time for today.
--
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