Re: [PATCH 1/9] tmp_objdir: add a helper function for discarding all contained objects
- From
Elijah Newren <newren@gmail.com>
- Date
- Dec 25, 2021, 02:29 UTC
- Message-ID
- <CABPp-BEMBkG=+g7SUXmJHqZ=qQQn-2oPg33uRtYyRHkt3SbxaA@mail.gmail.com>
- In-Reply-To
- <xmqqlf0dkvig.fsf@gitster.g>
On Tue, Dec 21, 2021 at 10:23 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 27 quoted lines
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> > +void tmp_objdir_discard_objects(struct tmp_objdir *t)
> >> > +{
> >> > + remove_dir_recursively(&t->path, REMOVE_DIR_KEEP_TOPLEVEL);
> >> > +}
> >> > +
> >>
> >> OK.
> >>
> >> Without a caller, it is a bit hard to judge if a separate helper
> >> makes the caller easier to read and understand, or becomes an extra
> >> layer of abstraction that obscures the logic. Hopefully, having a
> >> more specific function name with "tmp" and "discard" in it makes the
> >> intent at callers more clear than the function that is named after
> >> the detail of the operation.
> >
> > This isn't just a convenience; since tmp_objdir is defined in
> > tmp-objdir.c rather than tmp-objdir.h, t->path is not accessible
> > outside of tmp-objdir.c. Because of this, some kind of helper
> > function is necessary.
>
> But adding this function as an extra level of abstration is *not*
> the only way to expose the feature. Instead the internal of "struct
> tmp_objdir" could be exposed to the caller that wants to discard the
> files inside the path.Ah, yes, we talked about that during the tmp-objdir discussion back in September/October. Peff didn't want struct tmp_objdir exposed, and I was operating with that in mind.
> I think we now have enough material to fill between these two lines > to help readers ;-)
I've restructured the series a bit based on Ævar's feedback, and this function is now only introduced along with its caller. Hopefully that makes it a bit clearer.