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