Re: [PATCH 3/5] setup: merge configuration of repository formats
flat view
From: Justin Tobler <hidden>
Date: 2024-08-15 21:38:21
On 24/08/15 10:00AM, Patrick Steinhardt wrote:
The configuration of repository formats is split up across two functions `validate_hash_algorithm()` and `validate_ref_storage_format()`. This is fine as-is, but we are about to extend the logic to also read default values from the config. With the logic split across two functions, we would either have to pass in additional parameters read from the config, or read the config multiple times. Both of these options feel a bit unwieldy.
Combining the repository format configuration logic into a single function seems like a good option. It looks like go we even further though since we also include setting the hash and ref format for `the_repository`. Might be nice to also mention this.
Merge the code into a new a new function `repository_format_configure()`
s/new a new/new/
that is responsible for configuring the whole repository's format. Like this, we can easily read the config in a single place, only. Signed-off-by: Patrick Steinhardt <redacted> ---
[snip]
- /* - * Now that we have set up both the hash algorithm and the ref storage - * format we can update the repository's settings accordingly. - */
The above comment somewhat made it sound like the repository format had to be configured for both the hash algo and ref storage before updating `the_repository`, but that does not seem to be a requirement in actuality.
quoted hunk ↗ jump to hunk
- repo_set_hash_algo(the_repository, repo_fmt.hash_algo); - repo_set_ref_storage_format(the_repository, repo_fmt.ref_storage_format); + repository_format_configure(&repo_fmt, hash, ref_storage_format);
Overall, I like that this repostory format configuration is under `repository_format_configure()`. The `validate_*()` functions names confused me slightly initially because I assumed they were only validating, but they also configure the repo format. Looking good :) -Justin