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

Re: [PATCH v1 2/4] advice: add enum variants for missing advice variables

From
Ben Boeckel <mathstuf@gmail.com>
Date
Aug 3, 2021, 00:36 UTC
Message-ID
<YQiPq3gFnDa49UA7@erythro.dev.benboeckel.internal>
In-Reply-To
<nycvar.QRO.7.76.6.2108022333360.55@tvgsbejvaqbjf.bet>
On Mon, Aug 02, 2021 at 23:52:54 +0200, Johannes Schindelin wrote:
Show 16 quoted lines
> 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.

Show 23 quoted lines
> 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
Previous: Johannes SchindelinNext: Ben Boeckel
Message 6 of 32 in “advice: remove usage of `advice_*` global variables”
  1. 0/4 advice: remove usage of `advice_*` global variablesBen Boeckel, Jul 31, 2021
  2. 1/4 advice: add a function to set the value of an advice typeBen Boeckel, Jul 31, 2021
  3. Johannes SchindelinAug 2, 2021
  4. 2/4 advice: add enum variants for missing advice variablesBen Boeckel, Jul 31, 2021
  5. Johannes SchindelinAug 2, 2021
  6. Ben BoeckelAug 3, 2021
  7. 3/4 advice: remove uses of global `advice_` variablesBen Boeckel, Jul 31, 2021
  8. Johannes SchindelinAug 2, 2021
  9. Ben BoeckelAug 3, 2021
  10. 4/4 advice: remove static global variables for advice trackingBen Boeckel, Jul 31, 2021
  11. Johannes SchindelinAug 2, 2021
  12. Ben BoeckelAug 3, 2021
  13. Johannes SchindelinAug 2, 2021
  14. Ben BoeckelAug 3, 2021
  15. 0/4 advice: remove usage of `advice_*` global variablesBen Boeckel, Aug 5, 2021
  16. 1/4 advice: add enum variants for missing advice variablesBen Boeckel, Aug 5, 2021
  17. 2/4 advice: remove read uses of global `advice_` variablesBen Boeckel, Aug 5, 2021
  18. 3/4 advice: add `advice_set` to update advice settings at runtimeBen Boeckel, Aug 5, 2021
  19. 4/4 advice: remove static global variables for advice trackingBen Boeckel, Aug 5, 2021
  20. 0/4 advice: remove usage of `advice_*` global variablesÆvar Arnfjörð Bjarmason, Aug 6, 2021
  21. 2/4 advice: remove read uses of most global `advice_` variablesÆvar Arnfjörð Bjarmason, Aug 6, 2021
  22. Eric SunshineAug 6, 2021
  23. 1/4 advice: add enum variants for missing advice variablesÆvar Arnfjörð Bjarmason, Aug 6, 2021
  24. 3/4 advice: remove use of global advice_add_embedded_repoÆvar Arnfjörð Bjarmason, Aug 6, 2021
  25. Eric SunshineAug 6, 2021
  26. 4/4 advice: move advice.graftFileDeprecated squashing to commit.[ch]Ævar Arnfjörð Bjarmason, Aug 6, 2021
  27. 0/4 advice: remove usage of `advice_*` global variablesÆvar Arnfjörð Bjarmason, Aug 23, 2021
  28. 1/4 advice: add enum variants for missing advice variablesÆvar Arnfjörð Bjarmason, Aug 23, 2021
  29. 2/4 advice: remove read uses of most global `advice_` variablesÆvar Arnfjörð Bjarmason, Aug 23, 2021
  30. 3/4 advice: remove use of global advice_add_embedded_repoÆvar Arnfjörð Bjarmason, Aug 23, 2021
  31. 4/4 advice: move advice.graftFileDeprecated squashing to commit.[ch]Ævar Arnfjörð Bjarmason, Aug 23, 2021
  32. Junio C HamanoAug 25, 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.