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