On Fri, Feb 09, 2024 at 04:37:23PM +0800, Julien Rouhaud wrote:
> On Fri, Feb 09, 2024 at 03:38:23PM +0900, Yugo NAGATA wrote:
>> Also, I think the name is a bit confusing for the same reason, that is,
>> query_id_enabled may be false even when query id is computed in fact.
>>
>> Actually, this does not matter because we use IsQueryIdEnabled to check
>> if query id is enabled,  instead of referring to a global variable
>> (query_id_enabled or compute_query_id). But, just for making a code a bit
>> more readable, how about renaming this to query_id_required which seems to
>> stand for the meaning more correctly?
> 
> -1 for renaming to avoid breaking extensions that might access it.  We should
> simply document for compute_query_id and query_id_enabled declaration that one
> should instead use IsQueryIdEnabled() if they're interested in whether the 
> core
> queryid are computed or not.

Agreed.  A renaming would involve more pain than gain.  Improving the
comments around how to all that would be good enough, my 2c.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to