git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 0/3] Use default values from settings instead of config

From
Derrick Stolee <stolee@gmail.com>
Date
Oct 5, 2021, 17:43 UTC
Message-ID
<70aca052-716f-40ed-47c4-1882fdbd221e@gmail.com>
In-Reply-To
<87lf37ll4k.fsf@evledraar.gmail.com>
On 10/5/2021 7:57 AM, Ævar Arnfjörð Bjarmason wrote:
Show 24 quoted lines
> Subject: [PATCH] WIP gc/maintenance: just call prepare_repo_settings() earlier
> 
> Consolidate the various calls to prepare_repo_settings() to happen at
> the start of cmd_maintenance() (TODO: and cmd_gc()). This WIP commit
> intentionally breaks things, we seem to be lacking test coverage for
> cmd_gc(), perhaps since 31b1de6a09b (commit-graph: turn on
> commit-graph by default, 2019-08-13)?
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/gc.c            | 5 +----
>  t/t5318-commit-graph.sh | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 26709311601..f59dbedc1fe 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -695,7 +695,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  		clean_pack_garbage();
>  	}
>  
> -	prepare_repo_settings(the_repository);
>  	if (the_repository->settings.gc_write_commit_graph == 1)

I think that removing these calls is dangerous. prepare_repo_settings() already returns immediately if the repository already has its settings populated. The pattern of "call prepare before using a setting" is a safe way to future-proof the check from a movement of the call.

Putting that potential-future-problem aside, I don't see how this change is a _benefit_ other than fewer lines of code, which is not a quality measure in itself.

Thanks, -Stolee

Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 39 of 65 in “Use default values from settings instead of config”
  1. 0/3 Use default values from settings instead of configGlen Choo, Sep 13, 2021
  2. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Sep 13, 2021
  3. Taylor BlauSep 13, 2021
  4. Eric SunshineSep 13, 2021
  5. Taylor BlauSep 13, 2021
  6. Glen ChooSep 13, 2021
  7. Eric SunshineSep 13, 2021
  8. Taylor BlauSep 14, 2021
  9. Eric SunshineSep 14, 2021
  10. Taylor BlauSep 14, 2021
  11. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Sep 13, 2021
  12. Taylor BlauSep 13, 2021
  13. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Sep 13, 2021
  14. Taylor BlauSep 13, 2021
  15. Glen ChooSep 14, 2021
  16. Bagas SanjayaSep 14, 2021
  17. Glen ChooSep 16, 2021
  18. Taylor BlauSep 13, 2021
  19. 0/3 Use default values from settings instead of configGlen Choo, Sep 17, 2021
  20. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Sep 17, 2021
  21. Eric SunshineSep 29, 2021
  22. Glen ChooSep 30, 2021
  23. Glen ChooSep 30, 2021
  24. Eric SunshineSep 30, 2021
  25. Glen ChooOct 1, 2021
  26. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Sep 17, 2021
  27. Eric SunshineSep 29, 2021
  28. Glen ChooSep 29, 2021
  29. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Sep 17, 2021
  30. Eric SunshineSep 29, 2021
  31. Glen ChooSep 27, 2021
  32. Eric SunshineSep 29, 2021
  33. Glen ChooSep 29, 2021
  34. 0/3 Use default values from settings instead of configGlen Choo, Oct 5, 2021
  35. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 5, 2021
  36. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 5, 2021
  37. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 5, 2021
  38. Ævar Arnfjörð BjarmasonOct 5, 2021
  39. Derrick StoleeOct 5, 2021
  40. Ævar Arnfjörð BjarmasonOct 5, 2021
  41. Glen ChooOct 5, 2021
  42. Junio C HamanoOct 9, 2021
  43. Glen ChooOct 11, 2021
  44. Junio C HamanoOct 11, 2021
  45. Glen ChooOct 11, 2021
  46. 0/3 Use default values from settings instead of configGlen Choo, Oct 12, 2021
  47. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 12, 2021
  48. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 12, 2021
  49. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 12, 2021
  50. Junio C HamanoOct 12, 2021
  51. Ævar Arnfjörð BjarmasonOct 12, 2021
  52. Glen ChooOct 12, 2021
  53. Ævar Arnfjörð BjarmasonOct 14, 2021
  54. Derrick StoleeOct 13, 2021
  55. Ævar Arnfjörð BjarmasonOct 13, 2021
  56. Derrick StoleeOct 14, 2021
  57. Glen ChooOct 14, 2021
  58. Ævar Arnfjörð BjarmasonOct 14, 2021
  59. Junio C HamanoOct 14, 2021
  60. Junio C HamanoOct 15, 2021
  61. 0/3 Use default values from settings instead of configGlen Choo, Oct 15, 2021
  62. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 15, 2021
  63. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 15, 2021
  64. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 15, 2021
  65. Junio C HamanoOct 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.