Re: [PATCH v2 0/6] teach `worktree list` verbose mode and prunable annotations

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

 



Eric Sunshine writes:

> On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
> <rafaeloliveira.cs@xxxxxxxxx> wrote:
>> This patch series addresses some of these changes by teaching
>> `worktree list` to show "prunable" annotation, adding verbose mode and
>> extending the --porcelain format with prunable and locked annotation as
>> follow up from [1]. Additionally, it addresses one shortcoming for porcelain
>> format to escape any newline characters (LF or CRLF) for the lock reason
>> to prevent breaking the format that is mentioned in [4] and [1] during the
>> review cycle.
>
> Thanks for continuing to work on this. I'm pleased to see these
> enhancements coming together so nicely.
>
>> The patch series is organizes as:
>
> The new organization is a nice improvement over v1.
>
> As mentioned in my review of [5/6], there may be some value in
> swapping [5/6] and [6/6] to make it easier for readers to digest the
> changes, but it's not a hard requirement.
>
> To avoid being mysterious, there's also a change in [5/6] which
> probably belongs in [3/6], as explained in my review of [5/6].
>
> My review of patch [4/6] suggests optionally splitting out a bug fix
> into its own patch, but that's a very minor issue. Use your best
> judgment whether or not to do so.
>
>> 4. The fourth patch adds the "locked" attribute for the porcelain format
>>    in order to make both default and --porcelain format consistent.
>
> This patch does need a re-roll, and it's entirely my fault. In my
> review of the previous round, I said that if the lock reason contained
> special characters, we'd want to escape those characters and quote the
> string, but then I gave you a code suggestion which did the opposite
> of that. My [4/6] review goes into more detail.
>

I kindly disagree and I actually think is my fault :).

> Aside from the one important fix in [4/6], and the possible minor
> re-organizations suggested above, all my other review comments were
> very minor and probably subjective, thus nothing to sweat over.
>
> Nicely done.
>
> [1]: https://lore.kernel.org/git/CAPig+cSH6PKt8YvDJhMBvzvePYLqbf70uVV8TERoMj+kfxJRHQ@xxxxxxxxxxxxxx/

Thank you for reviewing this series and all the helpful suggestions. I
will send another revision taking all of them into account.

--
Thanks
Rafael



[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