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");
----------------
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).


https://reviews.llvm.org/D26511



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to