I think we have some misuse for global option in clang-tidy. Some checks use getLocalOrGlobal to get the option but I find some “global” option is only used in one check.
My current idea is we can only extract common used options as global options and make the other option as local options. The global option at least needs to be used in 3 checks IMO. (maybe we can discuss the number of this limitation later).
I think we should be careful here. Just because a name is repeated N times it does not mean it can be extracted out into a single global option (see comments in the PR).
Also, options are user-facing so we must design them carefully, as we cannot refactor them later on in an easy way, due to backwards compatibility.
One obvious candidate IMO to extracting into a global option is the IncludeStyle. It does not make sense for each check to have a different style.
Do note that we must do this in a progressive way to not break users. We first need support both local and global (should already be possible), deprecate the local one, and finally remove it after 2 releases.
Should we also give a deprecated warning for current global options StrictMode and IgnoreMacros and then move some to local only? I am worry about break lots of configuration directly.
A quick search on GitHub shows that IncludeStyle is often used on specific checks, what was the plan for the deprecation timeline of this being a global-only option? 1 or 2 deprecation releases? Code search results · GitHub
Either way, this plan sounds good (for IncludeStyle), thanks for driving this forward.
However, I don’t think that we would want to deprecate StrictMode and IgnoreMacros as local options. Having different settings per check for these is IMO quite valuable. E.g., the strict mode of one check may not fit a project’s needs, while another is exactly what is wanted. A similar (though weaker) argument can be made for IgnoreMacros.
Can we verify this? How many repos are using these options (wrongly) globally?
1 or 2 deprecation releases?
Yes, I believe in the current release we simply announce the deprecation and maintain support for both local and global, and in the next release we remove it. But depending on how much it’s used out there we may need more time to fully remove it.
I don’t think that we would want to deprecate StrictMode and IgnoreMacros as local options.
I agree, these make sense as local options, so the only change to be done is to replace getLocalOrGlobal with get.
My meaning is we do not support StrictMode and IgnoreMacros as global options. It must be set per check. For StrictMode, I think it is obviously to merge lots of different meaning to one global option. For IgnoreMacros, it depends IMO.
The global options means the option in -config="{CheckOptions: {StrictMode: true}}"
IMO it make sense. Because if someone find lots of IgnoreMacros, then it will extract it to global to reduce repeat. but for StrictMode, they will not because the detailing meaning of StrictMode depends on each check.
