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

Felipe Contreras [off-list ref] writes:

Junio C Hamano wrote:

quoted

Felipe Contreras [off-list ref] writes:

quoted

Junio C Hamano wrote:

quoted

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

Junio C Hamano wrote:

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

You started by saying these were "not the matter for your liking or not
liking", and now you are explaining why you like one better than the other.

I don't think it's particularly helpful to criticize one person's preferences
because they don't match yours.

You like one, I like another. That's it.

-- 
Felipe Contreras