Hi!
We have recently identified some refactoring opportunities that might impact users, for example moving check-specific options to global-scope (e.g. IncludeStyle).
Typically we handle this by documenting the deprecation in release N+1, and fully cleaning the code in release N+2.
However I believe it’s easy to miss this in the Release Notes, finding out about the deprecation when it’s already removed in N+2, which is not nice towards users.
Would it make sense for clang-tidy to print deprecation warnings as well? Then it would be much more obvious and would not go unnoticed. We could perhaps add a SuppressDeprecationWarnings option as well if people find it noisy.
3 Likes
Is there any best practice in clang / other llvm projects?
I don’t really know, how is it typically done for clang @AaronBallman ? Do you think it’s worth the effort, or are the Release Notes sufficient?
I’m thinking we don’t want to end up like this: clang-tidy broke the world by LebedevRI · Pull Request #1766 · google/benchmark · GitHub
But there the problem was a lot more severe (still unfixed).
1 Like
Sorry for the delayed response while I was out for the holidays.
In Clang, we’re somewhat inconsistent. For language constructs, we typically add a deprecation warning so users know they need to change their code. For compiler flags, we sometimes warn and we sometimes rely on the release notes, it’s kind of a value judgement based on how old the flag is and how much evidence of use there is of the flag in the wild.
Personally, I think we should err on the side of louder announcements when a change can plausibly be disruptive. Release notes are easy for users to miss; diagnostics are a bit less so.
I think we should go with the plan that @carlosgalvezp described. A hard-error is too disruptive, and could lead to a situation similar to that of AnalyzeTemporaryDtors. The deprecation of the option will also be mentioned twice, once when introducing the warning, and once when fully removing it, when it will cause a hard-error.
1 Like
Printing deprecation warnings makes sense to me. That’s what I meant by “louder announcements”, in case it wasn’t clear.
1 Like
It was 
I just wanted to explicitly say we should do it this way
1 Like
And maybe we should not do this in the last few days before the branch date, and just do this next release
2 Likes