[PATCH] D42261: [clang-tidy objc-property-declaration] New option IncludeDefaultAcronyms

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 22 01:02:11 PST 2018


hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:47
+
+.. option:: AdditionalAcronyms
+
----------------
benhamilton wrote:
> hokein wrote:
> > It seems to me the `Acronyms` and `AdditionalAcronyms` play most same role here. 
> > 
> > If we want to keep the long default list, how about using a bool flag `IncludeDefaultList` + the existing `Acronyms` option?
> > 
> > * if `IncludeDefaultList` is on, the acronyms will be "default" + "Acronyms".
> > * if `IncludeDefaultList` is off, the acronyms will be only "Acronyms".
> I think most people will want the "include default + add more" option. My goal was to make that possible, which is why I added the new `AdditionalAcronyms` option.
> 
> I agree the idea of a setting to control including the default list is nice, but I feel that should be on by default, since most users will want that.
> 
> If we add `IncludeDefaultList`, it should need to be on by default. That would break backwards compatibility for existing users. Do we think that's okay?
> 
> I'm assuming not a lot of people use this check yet, but I have no way of knowing that.
Yeah, `IncludeDefaultList` should be on by default.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42261





More information about the cfe-commits mailing list