From: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
Date: Sat, 10 Oct 2026 19:53:48 GMT
Subject: Re: [PATCH] repack: do not rebuild packs on --dry-run
Message-ID: <CAGWgyh9Qo9J6A+PWpwiZoZPQXDv+fgwRkPKT-bZbywWZgjO9sg@mail.gmail.com>
In-Reply-To: <CALnO6CD9roPhKsYRVXRuZXQckTmbCskKmvxiUyxp6vkJ03S8rQ@mail.gmail.com>

Hi Ben,

On Thu, 8 Oct 2026 at 18:26, D. Ben Knoble <ben.knoble@gmail.com> wrote:
> Just outside the patch context is the "if (dry_run)" conditional, so
> this is the right place. (In this case, formatting with a larger "-U"
> value might help.)
Thanks, Ben! I will do that in v2

> > +
> > +                       /*
> > +                        * add an exit here, so that dry run does not
> > +                        * go on to rebuild any pack or delete anything, even
> > +                        * if the user explicitly asked for -d
> > +                        */
>
> 2 notes:
>
> 1. "add an exit" will stop making sense as soon as the patch becomes a
> commit; that is, it only makes sense in the context of proposed
> changes. Once those changes are part of the code base, the comment and
> code are not adding anything. They simply are. So, if we need a
> comment (see 2), it might be best phrased as "exit here so that […]",
> keeping some of your original wording. Or we could be more terse:
> "skip non-dry-run operations" or something.
> 2. Do we need such a comment, I wonder? git-blame will point folks
> towards this commit :)
>
Oh yeah right, it will stop making sense without any context.
Yeah git blame does the thing, but I feel like adding a short comment for that

Will send a v2 soon

Thanks,
Siddharth