Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jun 3, 2025, 13:41 UTC
- Message-ID
- <aD77fz3N-fLI8DjM@pks.im>
- In-Reply-To
- <20250603131806.14915-1-ayu.chandekar@gmail.com>
On Tue, Jun 03, 2025 at 06:48:06PM +0530, Ayush Chandekar wrote:
Show 9 quoted lines
> diff --git a/repo-settings.c b/repo-settings.c > index 4129f8fb2b..406c70601c 100644 > --- a/repo-settings.c > +++ b/repo-settings.c > @@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r) > &r->settings.pack_use_bitmap_boundary_traversal, > r->settings.pack_use_bitmap_boundary_traversal); > repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1); > + repo_cfg_bool(r, "core.sparsecheckout", &r->settings.core_apply_sparse_checkout, 0);
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`.
Show 6 quoted lines
> @@ -227,3 +228,13 @@ void repo_settings_reset_shared_repository(struct repository *repo)
> {
> repo->settings.shared_repository_initialized = 0;
> }
> +
> +int repo_settings_get_apply_sparse_checkout(struct repository *repo)Same remark here -- where does the "apply" part come from?
Show 8 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()`.
Patrick