Run clang-format on ipc/glue

NEW
Assigned to

Status

()

Core
IPC
P3
normal
a year ago
6 months ago

People

(Reporter: billm, Assigned: billm)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

I think this makes sense to do. I made a quick pass over the patches and everything looks fine to me. The code in this directory has gone through a mix of styles, so the tool should help a lot.
Comment hidden (mozreview-request)
Comment on attachment 8811054 [details]
Bug 1317821 - Reformat ipc/glue with clang-format ()

https://reviewboard.mozilla.org/r/93296/#review93294

Interesting, there are some places where this doesn't match Gecko style - like missing braces on if-statements with single-line bodies. Also, not a fan of stuff like this:

 ~ShutdownObserver() { AssertIsOnMainThread(); }
 
Looks nicer, and is easier to step-through and modify as:

 ~ShutdownObserver() {
   AssertIsOnMainThread();
 }
 
But overall this looks like a big improvement.
Attachment #8811054 - Flags: review?(dvander) → review+

Comment 3

a year ago
mozreview-review
Comment on attachment 8811054 [details]
Bug 1317821 - Reformat ipc/glue with clang-format ()

https://reviewboard.mozilla.org/r/93296/#review93444

::: ipc/.clang-format:1
(Diff revision 1)
> +BasedOnStyle: Mozilla

Why you don't use .clang-format in the base directory?

We should like the number of different coding style in the base code.

Comment 4

a year ago
mozreview-review-reply
Comment on attachment 8811054 [details]
Bug 1317821 - Reformat ipc/glue with clang-format ()

https://reviewboard.mozilla.org/r/93296/#review93444

> Why you don't use .clang-format in the base directory?
> 
> We should like the number of different coding style in the base code.

s/like/limit/
Flags: needinfo?(wmccloskey)
Flags: needinfo?(dvander)
Flags: needinfo?(dvander)
(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Comment on attachment 8811054 [details]
> Bug 1317821 - Reformat ipc/glue with clang-format ()
> 
> https://reviewboard.mozilla.org/r/93296/#review93444
> 
> > Why you don't use .clang-format in the base directory?
> > 
> > We should like the number of different coding style in the base code.
> 
> s/like/limit/

The .clang-format file I'm checking in more closely matches Gecko style. Ideally we would use the ipc/.clang-format file in the rest of Gecko, but some of the directives in the ipc version require clang-format-4.0. I don't know who uses clang-format at the moment, so I'd rather not break anyone's workflow. Changing only ipc/ seems like a good first step.

Once this lands I'll post to dev-platform and see if anyone objects to requiring clang-format-4.0.
Flags: needinfo?(wmccloskey)
Depends on: 1322321
> Once this lands I'll post to dev-platform and see if anyone objects to requiring clang-format-4.0.
Thanks for the great input.

I updated  the root clang-format in bug 1322321 and push the change in clang upstream directly
https://reviews.llvm.org/rL289660

So, you can probably remove your local change in your patch.
Thanks again

Updated

6 months ago
Priority: -- → P3
Created attachment 8902869 [details] [diff] [review]
ipc.diff

here is what it looks like with the current set of rules
You need to log in before you can comment on or make changes to this bug.