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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
