alexfh added inline comments.
================ Comment at: clang-tidy/modernize/ModernizeTidyModule.cpp:58 "modernize-use-bool-literals"); - CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default"); + CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default"); CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace"); ---------------- alexfh wrote: > malcolm.parsons wrote: > > alexfh wrote: > > > aaron.ballman wrote: > > > > malcolm.parsons wrote: > > > > > aaron.ballman wrote: > > > > > > What do we want to do, if anything, for people who have scripts > > > > > > using the old name? Do we want to keep the old name as an alias to > > > > > > the new name for some period of time? > > > > > An alias helps if the check was enabled by name, but not if it was > > > > > disabled by name. > > > > > If the alias is temporary, would you want a deprecation warning? > > > > > I wouldn't want to warn about `-checks=modernize*`, but maybe warning > > > > > for `-checks=modernize-use-default` would be useful. > > > > > An alias helps if the check was enabled by name, but not if it was > > > > > disabled by name. > > > > > > > > Oye, this is true and unfortunate. > > > > > > > > > If the alias is temporary, would you want a deprecation warning? > > > > > I wouldn't want to warn about -checks=modernize*, but maybe warning > > > > > for -checks=modernize-use-default would be useful. > > > > > > > > I think a deprecation warning would be a helpful feature, but not > > > > required. I do agree that I would not want a warning for wildcard > > > > matches. > > > > > > > > I would also be fine if we simply had the documentation for > > > > `modernize-use-default` forward to the documentation for > > > > `modernize-use-equals-default` and put a note in there about the old > > > > name being deprecated and leave in an alias to the old name. > > > > > > > > To be complete, I would also be fine if we remove the old name as in > > > > this patch. I am mostly thinking about what default policy we want to > > > > have when this situation arises. FWIW, the check was exposed under this > > > > name around Oct 2015, so it's been in the wild for over a year, and in > > > > a public release. > > > I'd personally prefer to leave the old documentation file with a redirect > > > and a note about the renaming. Similar to how we treat aliases. WDYT? > > If it has a redirect then add_new_check.py will add it to list.rst using > > the same wording as an alias. > > Is that what you want? > > Should it be an alias? > I don't care about leaving the old name for the check (probably, better to > remove it right away), but leaving the old documentation page with a proper > redirect seems useful as a means to resolve users' confusion. I wouldn't call > it "alias" in this case though, and including it to the list doesn't seem to > be useful (so the document will need to be marked orphan, IIUC). (So, we can do something close to clang/tools/extra/docs/clang-tidy.rst) https://reviews.llvm.org/D26511 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits