Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

From: Ayush Chandekar <ayu.chandekar@gmail.com>
Date: Jun 3, 2025, 16:20 UTC
Message-ID: <CAE7as+Y0hzkPUC-q7dd-eSJVi0H_nwzQL1AGZJnKMyYcrX1dLw@mail.gmail.com>
In-Reply-To: <aD77fz3N-fLI8DjM@pks.im>

Show 5 quoted lines

> The config is called "core.sparseCheckout", so why is the variable
> called `core_apply_sparse_checkout`? `core_sparse_checkout` I would've
> understood, but where does "apply" come from? Also, for brevity I think
> we could just call it `settings.sparse_checkout`.
>

Yes, I had this thought as well that adding "apply" doesn't make a lot of sense. But I thought since the global variable has this name for a long time, there must have been some reason. I can change the name if the "apply" doesn't hold any value.

Show 19 quoted lines

>
> > +{
> > +     return repo->settings.core_apply_sparse_checkout;
> > +}
> > +
> > +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
> > +{
> > +     repo->settings.core_apply_sparse_checkout = value;
> > +}
>
> Getters and setters only really help in the case where they actually
> provide a benefit. These don't though, so it's dubious whether we should
> have them.
>
> Also, shouldn't these functions call `prepare_repo_settings()`?
> Otherwise we cannot guarantee that those settings have already been
> parsed at all. And for the setter it could happen that the settings get
> overwritten by the next caller of `prepare_repo_settings()`.
>

Oh, yeah, you're right. So, if we use `prepare_repo_settings()` in them, wouldn't it be better to use getter and setter functions? Otherwise, I'd have to call `prepare_repo_settings()` everywhere I'm using the setting.