Thread (83 messages) 83 messages, 3 authors, 2021-01-30

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

flat view

From: Rafael Silva <hidden>
Date: 2021-01-19 18:28:29

Eric Sunshine writes:
On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
[off-list ref] wrote:
quoted
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.
quoted
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.
quoted
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@mail.gmail.com/ (local)
Thank you for reviewing this series and all the helpful suggestions. I
will send another revision taking all of them into account.

--
Thanks
Rafael
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help