[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D86671#2243980 <https://reviews.llvm.org/D86671#2243980>, @njames93 wrote:

> Can you make sure you upload diffs with full context (-U99999). Or using arcanist it will be done automatically.
>
> Make sure the diff is up to date with trunk
>
> Remove any changes that aren't related to this patch, they just make this look noisy.

Hi @njames93,

1. It is my first time to use Phabricator and Arcanist, I will check how to make full context (-U99999).
2. Do you mean that always sync with the latest master?
3. OK.



================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:317
+  if (Style.Case == IdentifierNamingCheck::CaseType::CT_HungarianNotation) {
+    const auto TypePrefix = getHungarianNotationTypePrefix(Type.str(), Decl);
+    if (TypePrefix.length() > 0) {
----------------
Eugene.Zelenko wrote:
> Please don't use auto when type is not specified explicitly in same statement or iterator.
Got it, I fixed it in the next commit. Thank you.


================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:430
+    const NamedDecl *pNamedDecl = dyn_cast<NamedDecl>(pDecl);
+    const auto TypePrefix =
+        getHungarianNotationTypePrefix(Type.str(), pNamedDecl);
----------------
Eugene.Zelenko wrote:
> Please don't use auto when type is not specified explicitly in same statement or iterator.
Got it, I fixed it in the next commit. Thank you.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting naming check with Hungarian notation.
+
----------------
Eugene.Zelenko wrote:
> Please move to `Changes in existing checks` section (in alphabetical order).
> 
> I think statement should describe user-facing changes.
OK~ Fix it in next commit.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:76
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   <clang-tidy/checks/readability-identifier-naming>` check.
----------------
Eugene.Zelenko wrote:
> Unrelated change.
OK~ Fix it on next commit.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86671/new/

https://reviews.llvm.org/D86671