Thread (1 message) 1 message, 1 author, 2020-02-17

Re: [PATCH v2 0/2] [RFC][Outreachy] advice: refactor advise API

From: Junio C Hamano <hidden>
Date: 2020-02-17 00:10:06

"Heba Waly via GitGitGadget" [off-list ref] writes:
Main changes in V2: 1- Rename advise_ng to advise_if_enabled. 2- Add a new
advise_enabled() helper. 3- Add a list of config variables names to replace
advice_config[] (used by list_config_advices()). 4- Send an enum parameter
to the new advise helpers instead of strings. 5- Extract vadvise() from
advise() and advise_if enabled().

Nice.  There is not much point for me to correct mistakes in the
title of the cover letter, but anyway... I think these changes are
not "refactoring" the API, but these are enhancing, extending or
even revamping the API.  
To introduce a new advice message, the caller needs to:

 * Add a new_advice_type to 'enum advice_type'
 * Come up with a new config variable name and add this name to 
   advice_config_keys[]
 * Call advise_if_enabled(new_advice_type, "advice message to be printed")
 * Or call advice_enabled(new_advice_type) first and then follow is by
   advice("advice message to be printed") as explained earlier.
 * Add the new config variable to Documentation/config/advice.txt
And I see that now you are going all the way to discard the
string-based keys to enumeration, I think this deserves to be called
"revamp advise API".
In the future, we can investigate generating the documentation from the list
of config variables or vice versa to make introducing a new advice much
easier, but this approach will do it for now.
Yup.  One step at a time.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help