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

Mon Sep 7 10:09:00 PDT 2020

njames93 added a comment.

In D86671#2259443 <https://reviews.llvm.org/D86671#2259443>, @dougpuob wrote:

> In D86671#2259364 <https://reviews.llvm.org/D86671#2259364>, @njames93 wrote:
>
>> Did you upload this incorrectly again, context is missing and seems to be a relative diff from a previous version of this patch?
>
> Sorry for it, I think it's my bad. It is possible that I manually merged the last master(github) with changes then updated them both via web interface ...
>
> Can I fix it if switch back to the base (`14948a0`) then merge all my changes, then update the diff again via web interface? Or do you have any better suggestion?
>
> I am curious about how do you know this mistake? You got error messages with `arc patch D86671` ?

The no context is easy to spot as phab says context not available. Its easy to find knowing that there is no mention of hungarian notation in the trunk version of IdentifierNamingCheck.cpp, yet there is mention of that in the before diff.

The way I do my patches is I create a branch from the current master. Then all commits go into that branch. When its time to update the PR I can just do a diff from <feature_branch> to <master>.
Though I do use arcanist for my patches

  arc diff master

arcanist will check to see if the current branch has tags for PR and automatically update that PR. Otherwise it will create a new PR.
If it goes to create a new PR instead of updating an existing one you can pass update

  arc diff master --update D86671

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671