carlosgalvezp added a comment. Thanks for the input!
> release note mentions Sure thing! > documentation changes As we discussed in the separate mail thread, there is no documentation at all as to what default values an alias/main check has. Where should I put that? Or do you mean that now that all 3 checks (primary + 2 aliases) have identical defaults, I should document that in the main check documentation? > test coverage I was surprised to not find any unit test for this check, probably because it's already done by the unit test of the "main check"? Should add a new unit test, pretty much copy-pasting the test for the main check? > evidence that this option is not being used by users Not sure how to go about this one :) I guess we can't have a 100% guarantee that this won't break things for users, there will always be an edge case. Perhaps we should document it very clearly in the Release Notes as well as explaining how to revert to the old behavior if needed? Since we are talking about a "guideline check" (cppcoreguidelines-explicit-virtual-functions) and a "good practice check" (modernize-use-override), I don't think it's unreasonable that users have enabled both checks. If so, the "modernize" check should already have this behavior as default and prompted users to make changes - the cppcoreguidelines was less strict, and now it would be as strict as the "modernize-use-override" check. In that scenario, this patch shouldn't break things for users. The scenario that would break things for users is when they only enable the cppcoreguidelines check alone. The guidelines changed in 2019, I think after 2 years users wouldn't mind the tooling catching up :) Since I'm new here and missed quite a few things, do you know if there's a checklist somewhere where I can read about what else I should include in the patch? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111041/new/ https://reviews.llvm.org/D111041 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
