| Message ID | 20210731022504.1912702-3-mathstuf@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | advice: remove usage of `advice_*` global variables | expand |
Hi Ben, On Fri, 30 Jul 2021, Ben Boeckel wrote: > These were missed in their addition in 887a0fd573 (add: change advice > config variables used by the add API, 2020-02-06). All other global > variable settings have entries already. It took quite a bit of reading and looking through the `git log` history to piece together what is going on here, and I wish the commit message would have explained this better. A big puzzlement came from the claim that "These were missed" is not only missing a noun that clarifies what "These" are meant to be, but also from the fact that `git grep advice_setting 887a0fd573` comes up empty. Which suggests to me that nothing was missed there, but the problem lies with `hw/advise-ng`, merged via c4a09cc9ccb (Merge branch 'hw/advise-ng', 2020-03-25), is based on v2.25.0, but was only merged after v2.26.0, which contains daef1b300b0 (Merge branch 'hw/advice-add-nothing', 2020-02-14). In other words, the addition of the two entries `addEmptyPathspec` and `addIgnoredFile` happened in a diverging branch from the addition of the `advice_setting` array, and the problem lies with the merge of the latter into a branch that already had merged the former. It would have helped me to read something along these lines: In daef1b300b0 (Merge branch 'hw/advice-add-nothing', 2020-02-14), two advice settings were introduced into the `advice_config` array. Subsequently, c4a09cc9ccb (Merge branch 'hw/advise-ng', 2020-03-25) started to deprecate `advice_config` in favor of a new array, `advice_setting`. However, the latter branch did not include the former branch, and therefore `advice_setting` is missing the two entries added by the `hw/advice-add-nothing` branch. These are currently the only entries in `advice_config` missing from `advice_setting`. FWIW I manually verified that last paragraph's claim. The diff itself looks correct to me. Ciao, Dscho
On Mon, Aug 02, 2021 at 23:52:54 +0200, Johannes Schindelin wrote: > On Fri, 30 Jul 2021, Ben Boeckel wrote: > > These were missed in their addition in 887a0fd573 (add: change advice > > config variables used by the add API, 2020-02-06). All other global > > variable settings have entries already. > > It took quite a bit of reading and looking through the `git log` history > to piece together what is going on here, and I wish the commit message > would have explained this better. > > A big puzzlement came from the claim that "These were missed" is not only > missing a noun that clarifies what "These" are meant to be, but also from > the fact that `git grep advice_setting 887a0fd573` comes up empty. Which > suggests to me that nothing was missed there, but the problem lies with > `hw/advise-ng`, merged via c4a09cc9ccb (Merge branch 'hw/advise-ng', > 2020-03-25), is based on v2.25.0, but was only merged after v2.26.0, which > contains daef1b300b0 (Merge branch 'hw/advice-add-nothing', 2020-02-14). Ah, I missed the logical merge conflict that was in effect here. I'll add this to the commit message. > In other words, the addition of the two entries `addEmptyPathspec` and > `addIgnoredFile` happened in a diverging branch from the addition of the > `advice_setting` array, and the problem lies with the merge of the latter > into a branch that already had merged the former. > > It would have helped me to read something along these lines: > > In daef1b300b0 (Merge branch 'hw/advice-add-nothing', 2020-02-14), > two advice settings were introduced into the `advice_config` > array. > > Subsequently, c4a09cc9ccb (Merge branch 'hw/advise-ng', > 2020-03-25) started to deprecate `advice_config` in favor of a new > array, `advice_setting`. > > However, the latter branch did not include the former branch, and > therefore `advice_setting` is missing the two entries added by the > `hw/advice-add-nothing` branch. > > These are currently the only entries in `advice_config` missing > from `advice_setting`. > > FWIW I manually verified that last paragraph's claim. I did as well :) ("All other global variable settings have entries already."). I also verified the reverse, but that was going to be moot with the other patches anyways. But having it called out as a separate paragraph sounds better. Thanks, --Ben
diff --git a/advice.c b/advice.c index fd58631dc1..fd9634aa4f 100644 --- a/advice.c +++ b/advice.c @@ -106,6 +106,8 @@ static struct { int enabled; } advice_setting[] = { [ADVICE_ADD_EMBEDDED_REPO] = { "addEmbeddedRepo", 1 }, + [ADVICE_ADD_EMPTY_PATHSPEC] = { "addEmptyPathspec", 1 }, + [ADVICE_ADD_IGNORED_FILE] = { "addIgnoredFile", 1 }, [ADVICE_AM_WORK_DIR] = { "amWorkDir", 1 }, [ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] = { "checkoutAmbiguousRemoteBranchName", 1 }, [ADVICE_COMMIT_BEFORE_MERGE] = { "commitBeforeMerge", 1 }, diff --git a/advice.h b/advice.h index 74425a9f1a..101c4054b7 100644 --- a/advice.h +++ b/advice.h @@ -45,6 +45,8 @@ extern int advice_add_empty_pathspec; */ enum advice_type { ADVICE_ADD_EMBEDDED_REPO, + ADVICE_ADD_EMPTY_PATHSPEC, + ADVICE_ADD_IGNORED_FILE, ADVICE_AM_WORK_DIR, ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME, ADVICE_COMMIT_BEFORE_MERGE,
These were missed in their addition in 887a0fd573 (add: change advice config variables used by the add API, 2020-02-06). All other global variable settings have entries already. Signed-off-by: Ben Boeckel <mathstuf@gmail.com> --- advice.c | 2 ++ advice.h | 2 ++ 2 files changed, 4 insertions(+)