Re: [PATCH v6 3/6] set-head: better output for --auto

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

 



"Bence Ferdinandy" <bence@xxxxxxxxxxxxxx> writes:

> On Fri Oct 11, 2024 at 22:43, karthik nayak <karthik.188@xxxxxxxxx> wrote:
>> "Bence Ferdinandy" <bence@xxxxxxxxxxxxxx> writes:
>>
>> [snip]
>>
>>>>
>>>> > +		git remote set-head --auto origin >output &&
>>>> > +		echo "'\''origin/HEAD'\'' is now created and points to '\''main'\''" >expect &&
>>>>
>>>> Nit: might be cleaner to use `${SQ}` here and below
>>> You mean something like this?
>>>
>>> 	git remote set-head --auto origin >output &&
>>> 	HEAD="'\''origin/HEAD'\''" &&
>>> 	echo "${HEAD} is now created and points to '\''main'\''" >expect &&
>>>
>>> I tried a few variations and this is what I could get working, but I may be
>>> simply missing something with the backtick.
>>
>> I mean simply this
>>
>>     git remote set-head --auto origin >output &&
>>     echo "${SQ}origin/HEAD${SQ} is now created and points to
>> ${SQ}main${SQ}" >expect &&
>
> Ah, I see in other tests this is used, but not in this particular test file.
> It's a bit hard to decide which is more cryptic, but ${SQ} is nicer on the
> eyes. On the other hand I would either switch the entire file in a separate
> patch or leave in the '\'' here as well. Or I guess one could go through the
> entire test base and switch everything to either one or the other for
> consistency.

I'm not sure I entirely agree with this sentiment. Consistency is a
great goal to target, but it shouldn't hinder changes that are
beneficial. In our case, if you make the first change, there is now
reason for upcoming changes to the same file to also use '${SQ}' and
eventually we can reach consistency of using '${SQ}' throughout the file.

- Karthik

Attachment: signature.asc
Description: PGP signature


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

  Powered by Linux