Wizard marked 4 inline comments as done.
Wizard added a comment.

In https://reviews.llvm.org/D45392#1060485, @Eugene.Zelenko wrote:

> If this is Apple guideline, check name should reflect this. I think will be 
> good idea to have general check for Apple naming conventions instead of 
> separate checks for specific situations like //objc-ivar-declaration// and 
> //objc-property-declaration//.


Thanks for the suggestion. I understand your point that they are both naming 
convention, however, they are about different components and using totally 
different naming rules. PropertyDeclarationCheck is already a very complicated 
check (the most complicated one for ObjC), I would rather not make it more 
heavy and try my best to split independent logic to different checks.



================
Comment at: clang-tidy/objc/IvarDeclarationCheck.cpp:23
+
+FixItHint generateFixItHint(const ObjCIvarDecl *Decl) {
+  auto IvarName = Decl->getName();
----------------
Eugene.Zelenko wrote:
> Please use static instead of anonymous namespace.
Using anonymous namespace was suggested by others for private methods that are 
only used within the check (e.g. ForbiddenSubclassingCheck, 
PropertyDeclarationCheck, etc). I would rather keep this pattern consistent 
with other checks.


================
Comment at: docs/ReleaseNotes.rst:60
 
+- New :doc:`objc-ivar-declaration
+  <clang-tidy/checks/objc-ivar-declaration>` check
----------------
Eugene.Zelenko wrote:
> Please place in new check list in alphabetical order.
I did not see a "new check list" in alphabetical order in this file. I believe 
they are ordered by commit time.If you mean the list.rst, they are already 
ordered alphabetically.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45392



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

Reply via email to