On Sun, Feb 26 2023, Jeff King wrote: > On Thu, Feb 09, 2023 at 12:24:14PM -0800, Junio C Hamano wrote: > >> Instead of forcing the porcelain commands to always read the >> configuration variables related to the signing and verifying >> signatures, lazily initialize the necessary subsystem on demand upon >> the first use. >> >> This hopefully would make it more future-proof as we do not have to >> think and decide whether we should call git_gpg_config() in the >> git_config() callback for each command. > > Sorry, I seem to have missed this when you originally posted it. And I > saw it marked as "will merge to next?" in the latest what's cooking. It > looks good to me, and I think we can proceed with it (though of course > it is not urgent and can probably wait until post-2.40). > >> A few git_config() callback functions that used to be custom >> callbacks are now just a thin wrapper around git_default_config(). >> We could further remove, git_FOO_config and replace calls to >> git_config(git_FOO_config) with git_config(git_default_config), but >> to make it clear which ones are affected and the effect is only the >> removal of git_gpg_config(), it is vastly preferred not to do such a >> change in this step (they can be done on top once the dust settled). > > Yes, I think it is good not to do so in this patch. If we want to do it > now on top, here's a patch. Though I could also see the argument for > just leaving them. > > -- >8 -- > Subject: [PATCH] drop pure pass-through config callbacks > > Commit fd2d4c135e (gpg-interface: lazily initialize and read the > configuration, 2023-02-09) shrunk a few custom config callbacks so that > they are just one-liners of: > > return git_default_config(...); > > We can drop them entirely and replace them direct calls of > git_default_config() intead. This makes the code a little shorter and > easier to understand (with the downside being that if they do grow > custom options again later, we'll have to recreate the functions). > > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > I looked over the output of: > > git grep --function-context 'return git_default_config' > > to see if there were other cases, not caused by fd2d4c135e. But I didn't > see any. As added review: This is the same patch diff as I sent on February 9th: https://lore.kernel.org/git/patch-1.2-d93c160dcbc-20230209T142225Z-avarab@xxxxxxxxx/; my local range-diff to my previously submitted topic & next being: 229: cc5d1d32fd4 ! 2: d93c160dcbc drop pure pass-through config callbacks @@ ## Metadata ## -Author: Jeff King <peff@xxxxxxxx> +Author: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> ## Commit message ## - drop pure pass-through config callbacks + {am,commit-tree,verify-{commit,tag}}: refactor away config wrapper - Commit fd2d4c135e (gpg-interface: lazily initialize and read the - configuration, 2023-02-09) shrunk a few custom config callbacks so that - they are just one-liners of: + In the preceding commit these config functions became mere wrappers + for git_default_config(), so let's invoke it directly instead. - return git_default_config(...); - - We can drop them entirely and replace them direct calls of - git_default_config() intead. This makes the code a little shorter and - easier to understand (with the downside being that if they do grow - custom options again later, we'll have to recreate the functions). - - Signed-off-by: Jeff King <peff@xxxxxxxx> - Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> ## builtin/am.c ## @@ builtin/am.c: static int parse_opt_show_current_patch(const struct option *opt, const char *ar -: ----------- > 3: c099d48b4bf gpg-interface.c: lazily get GPG config variables on demand So this LGTM.
