Re: [PATCH v2 0/6] teach `worktree list` verbose mode and prunable annotations
- From
Rafael Silva <rafaeloliveira.cs@gmail.com>
- Date
- Jan 19, 2021, 16:44 UTC
- Message-ID
- <gohp6klfco6fmk.fsf@gmail.com>
- In-Reply-To
- <CAPig+cTGxOwUN20JmeR0bBwVMM78eA3Bnaqei5GEAs2XFM2_gg@mail.gmail.com>
Eric Sunshine writes:
Show 37 quoted lines
> On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva > <rafaeloliveira.cs@gmail.com> 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 :).
Show 7 quoted lines
> 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@mail.gmail.com/
Thank you for reviewing this series and all the helpful suggestions. I will send another revision taking all of them into account.
-- Thanks Rafael