Re: [PATCH v3 1/5] pull: rename pull.rename to pull.mode

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

 



On Fri, Oct 11, 2013 at 8:25 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Fri, Oct 11, 2013 at 08:15:46PM -0500, Felipe Contreras wrote:
>
>> >> You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in the
>> >> meantime no shell script does that, and that's no reason to reject this patch
>> >> series.
>> >
>> > You are completely missing Matthieu's point that we attempt to be
>> > consistent in the format of messages, as well as where they are output,
>> > and from a user's perspective it does not matter what language the tool
>> > is implemented in.
>>
>> If we truly did that, there should be a warning () function, like in C.
>
> Or people could hand-code them to look similar, which is exactly what
> has happened.

And by doing that be prone to make mistakes, like using 'WARNING',
instead of 'warning'. But I guess you don't care about consistency
_that much_.

> If you want to factor out a warning function to clean up the code, be my
> guest. But the lack of one does not provide an argument that you should
> break consistency.

Consistency is already broken.

>> > -               echo "The configurations pull.rebase and branch.<name>.rebase are deprecated."
>> > -               echo "Please use pull.mode and branch.<name>.pullmode instead."
>> > +               echo >&2 "warning: The configurations pull.rebase and branch.<name>.rebase are deprecated."
>> > +               echo >&2 "Please use pull.mode and branch.<name>.pullmode instead."
>> [...]
>>
>> Are you sure you want me to squash that in? Because the warnings
>> wouldn't be consistent. Some would be "WARNING: " and others would be
>> "warning: ". Personally I don't care, but if your argument is
>> consistency, you should. If we had a warning () function, we could
>> truly be consistent.
>
> It is significant in the most important ways, which are labeling it at
> all, and sending it to stderr. Capitalization is less important, in my
> opinion.

It's still inconsistent.

> Using a lowercase version is much more consistent with the warnings
> produced by C code, which is why I chose it over the capitalized
> version. Again, if you want to change the existing WARNING cases in the
> shell scripts to be consistent with C output, be my guest.

It seems you are not that interested in consistency after all.

> Do you actually have some reason for wanting to output to go to stdout?

I'm fine with 'echo "warning: foo" >&2', but still, if you really
cared about consistency, there would be a warn() function, precisely
to avoid the mistakes of WARNING vs. warning which are already there,
plus future ones.

-- 
Felipe Contreras
--
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]