From: Siddharth Asthana Date: Sun, 20 Nov 2022 07:42:06 GMT Subject: Re: [PATCH v4 0/3] Add mailmap mechanism in cat-file options Message-ID: <42aa5823-36d7-8860-90ab-953eb6d2f2b4@gmail.com> In-Reply-To: On 14/11/22 23:18, Christian Couder wrote: > On Sun, Nov 13, 2022 at 10:28 PM Siddharth Asthana > wrote: > >> In this patch series we didn't want to change that '%(objectsize)' >> always shows the size of the original object even when `--use-mailmap` >> is set because first we have the long term plan to unify how the formats >> for `git cat-file` and other commands works. And second existing formats >> like the "pretty formats" used bt `git log` have different options for > > s/used bt/used by/ > >> fields respecting mailmap or not respecting it (%an is for author name >> while %aN for author name respecting mailmap). > > [...] > >> = Patch Organization >> >> - The first patch makes `-s` option to return updated size of the >> object, when combined with `--use-mailmap` option, after >> replacing the idents using the mailmap mechanism. >> - The second patch makes `--batch-check` option to return updated size of >> the object, when combined with `--use-mailmap` option, >> after replacing the idents using the mailmap mechanism. >> - The third patch improves the documentation of `-s`, `--batch`, >> `--batch-check` and `--batch-command` options by adding they can be >> combined with `--use-mailmap` options. > > So the documentation patch is now part of this small series again. > > Even if this documentation patch is a bug fix, it might be better at > this point to squash this patch into the patches 1/3 and 2/3. At least > I think that would better follow Junio's last comments about this. If > you go this way, you might want to squash the documentation parts > about -s from patch 3/3 into patch 1/3 and the rest of patch 3/3 into > patch 2/3. > >> = Changes in v4 >> >> - Improve the documentation patch to clearly state that the `-s`, >> `--batch-check`, `--batch-command` and `--batch` options can be only >> be used with `--textconv`, `--filters` or `--use-mailmap`. > > Here I think you should say that the documentation patch is part of > this series again and explain a bit why. > > Anyway I took a look at the actual patches in this series and they > look good to me now. So I would be Ok to merge it either as is or with > patch 3/3 squashed into patches 1/3 and 2/3 as discussed above. > > Thanks! Thanks a lot for the review Christian :) I have made the suggested changes in v5.