Closed
Bug 1293528
Opened 8 years ago
Closed 8 years ago
clang-format gtests
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.27
People
(Reporter: mt, Assigned: mt)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
2.83 KB,
patch
|
franziskus
:
review+
|
Details | Diff | Splinter Review |
412.90 KB,
patch
|
franziskus
:
review+
|
Details | Diff | Splinter Review |
This one is different to other code. We've been using Google style for our tests. We should make that formal.
Assignee | ||
Comment 1•8 years ago
|
||
Up on rietveld: https://codereview.appspot.com/302390043/ In this case, I saw no point in deviating from Google style.
Comment 2•8 years ago
|
||
lgtm, but I don't see a .clang-format file in the diff?
Flags: needinfo?(martin.thomson)
Assignee | ||
Comment 3•8 years ago
|
||
dammit rietveld: it looks like this: $ cat external_tests/.clang-format --- Language: Cpp BasedOnStyle: Google ...
Flags: needinfo?(martin.thomson)
Assignee | ||
Comment 4•8 years ago
|
||
Base patch (no formatting applied).
Assignee | ||
Comment 5•8 years ago
|
||
The patch that will rot very quickly.
Assignee: nobody → martin.thomson
Updated•8 years ago
|
Attachment #8779570 -
Flags: review+
Comment 6•8 years ago
|
||
Comment on attachment 8779571 [details] [diff] [review] clang_format_gtest-2.patch Review of attachment 8779571 [details] [diff] [review]: ----------------------------------------------------------------- I didn't check details, but looks good to me. Looks like the google style sorts includes. Something we explicitly disabled for the rest of the code base. That's fine with me but we should be aware of it.
Attachment #8779571 -
Flags: review+
Assignee | ||
Comment 8•8 years ago
|
||
Rebasing is easy. https://hg.mozilla.org/projects/nss/rev/06f9767b53cb https://hg.mozilla.org/projects/nss/rev/bd032bda5d0a Note that you can now run the following to make sure that your code is clang-format clean: $ automation/taskcluster/scripts/run_clang_format.sh --apply
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(martin.thomson)
Resolution: --- → FIXED
Target Milestone: --- → 3.27
You need to log in
before you can comment on or make changes to this bug.
Description
•