[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC

Mon Jul 2 10:00:27 PDT 2018

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

In https://reviews.llvm.org/D48716#1149293, @jolesiak wrote:

> General comment to changes https://reviews.llvm.org/D48716, https://reviews.llvm.org/D48718, https://reviews.llvm.org/D48719, https://reviews.llvm.org/D48720 (which are the split of https://reviews.llvm.org/D48352):

> These changes are separate, in the sense that they fix different issues. However, they should be chained as every change (apart from the first one) is based on previous ones: https://reviews.llvm.org/D48716 -> https://reviews.llvm.org/D48718 -> https://reviews.llvm.org/D48719 -> https://reviews.llvm.org/D48720. I don't know how to chain them in Phabricator (I should probably leave some comments about what every change is based on though).

Phabricator has an `Edit Related Revisions` feature where you can tag other revisions as being dependencies of or depending upon the current revision:

F6560886: Screen Shot 2018-07-02 at 10.57.37 AM.png <https://reviews.llvm.org/F6560886>

I went ahead and used it to link the revisions in the order you listed above. (I also like to edit the 1-line description and add `[1/x]` at the beginning — but that's up to you.)

> https://reviews.llvm.org/D48716 fixes the mechanism of counting parameters. This is an internal change though and doesn't influence formatting on its own (at the current state). Its lack would be visible after applying https://reviews.llvm.org/D48719. Therefore, I'm not aware of any formatting test that fails before applying this change and succeeds after. As far as I know internal functions/methods are not tested at all. Thus, the tests are part of https://reviews.llvm.org/D48719.

Great. Just mentioning that in the diff description should be fine.

Repository:
  rC Clang

https://reviews.llvm.org/D48716