Re: [PATCH 0/5] fc/transport-helper-sync-error-fix rebased

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

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

> Junio C Hamano wrote:
>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>> 
>> > Junio C Hamano wrote:
>> >> As I have said in the recent What's cooking reports, the original
>> >> posted here were based on older codebase and needed to be rebased,
>> >> but it had some conflicts and I wanted to see the result double
>> >> checked by the original author before we can merge it to 'next',
>> >> cooked there and hopefully merged to 'master' before tagging -rc1.
>> >> 
>> >> So here is the series that has been queued in 'pu' for the past
>> >> several days.
>> >> 
>> >> Felipe, can you double check it?
>> >
>> > These patches don't help much,...
>> 
>> What do you mean by that, exactly?  As long as your "don't help
>> much" is "would not hurt and will help some even for a small subset
>> of audience", that would be OK, but I am puzzled.
>
> I mean if purpose of these was for me to review the changes you did, it doesn't
> help as much as an interdiff does.

Ah, I misunderstood.  The thing was, that the original was based on
older codebase and I couldn't apply them to my tree cleanly.  It
would be unfair for you to expect _me_ to give you an interdiff in
such a situation, especially when I am asking you to double check
the conflict resolution based on that original.

OK, so I'll advance the topic to the 'next' and hopefully we can
merge it to 'master' before rc1.

> There's nothing in Documentation/CodingGuidelines that says multiple piped
> commands should be one on each line even if togther the line doesn't exceed 80
> characters, nor does it says that file names should be between quotes, even if
> the file name can't possibly have spaces.

Heh, we don't spell out every possible rules. That is where "do as
surrounding code" comes in.

As to "$1" vs $1 without quotes, I had to check the calling sites of
clean_mark and cmp_marks, primarily because these functions do not
say what their "$1" is, to see if an unquoted form is safe.  The
next person who reads the script has to do the same and quoting is
an obvious way to avoid it.

If the functions said "$1 is a branch name", it would have been
clear that unquoted $1 would be OK, but still, what these functions
has to take (especially the "clean" one that takes pathnames) can
change in the future, and a quoted form is an obvious way to
futureproof and relieve readers and programmers from having to worry
about the quoting issues.

So I think it is better to quote these in this particular case.

The pipe is purely subjective readability. I can go either way, but
since I was the one who was applying the patch that needed other
changes anyway... ;-)

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