Re: [PATCH] commit: Remove backward goto in read_craft_line()

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

 



I remember about a peoble (not on this project) who sad that
also a little change like this should includes in the sources because
it's make the code better without fixing a bug or add a new feature.
But in my experience these patches are not very popular.
But sure, I understand your reasons, Junio.
The statement from above is not even true.

2010/12/1 Junio C Hamano <gitster@xxxxxxxxx>:
> Ralf Thielow <ralf.thielow@xxxxxxxxxxxxxx> writes:
>
>>> If "--show-c-function" output is the problem, perhaps we should know a bit
>>> better about what C function header looks like?
>>
>> In fact the "--show-c-function" output is the problem. But I think that
>> a change can't be rejected because of another issue.
>
> Well, I never said anything about rejection nor acceptance.
>
>> The style of placing "goto"-statements, which leave a function to the
>> end of that is used in many other projects. And I think
>> it's very usefull.
>
> It is a good discipline to follow in general to place an exceptional case
> at the end if you jump out of the general flow. ÂBut because the affected
> function was so small, its readability doesn't benefit very much from the
> discipline (in other words, you knew about a good discipline, but applied
> it to a wrong function). ÂThe patch was small and obviously correct, so it
> will not hurt, but it will not make the world greatly a better place,
> either.
>
> IOW, it was a "Meh" topic for me.
>
> It was more important to discuss Jonathan's "leave SP at the beginning of
> a goto label to please --show-c-function" from the maintainer's point of
> view, as people may remember it as a rule and start sending patches that
> follow it, which I will need to deal with in the future. ÂI do not think
> that one is a good rule.
>
> Now that we have dealt with that more important business of letting people
> know that protecting goto labels with a leading SP is _not_ the rule ;-),
> I am happy with this discussion.
>
> And often I forget about the original issue when a discussion reaches this
> stage of happiness. ÂSo thanks for reminding me.
>
> As I said, even though I do not think the particular function you touched
> badly needs the discipline applied, it would not hurt, and it obviously is
> the right thing to do (iow, if the function were written from day one in a
> way your patch reorganized it, we would never accept a patch to change it
> into today's shape of jumping backwards). ÂFor one thing, it would remove
> an example from the codebase people can point at to make excuses when
> responding to a review of their patch that adds a backward goto to a much
> larger function.
>
> Will queue after massaging the log message somewhat. ÂThanks.
>
--
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]