djasper requested changes to this revision.
djasper added a comment.
This revision now requires changes to proceed.
So, while it might be convenient to view this all in one file, a test here is
not convenient for me (or presumably other clang-format developers) to work
with. You can make a prett
sylvestre.ledru added a comment.
> What problem is this addressing, though? Have we frequently changed
> formatting in a way that has negatively impacted Mozilla code?
We are integrating more clang-format into gecko processes.
About the reason, I believed I explained them in the comment above
djasper added a comment.
Please don't add this as is. I don't usually run the file-based tests in my
development workflow and suspect that I might be breaking this a lot.
If you want something like this, please add it as unittest(s) in
unittests/Format/... (either in a new file or in an existin
sylvestre.ledru added a comment.
This isn't about testing individual issues. This is to make sure that the
Mozilla coding style remains consistent and we don't regress specific coding
style. I found that easy and simple to have a single file to check for the
specificities.
After https://review
krasimir added a comment.
My input:
Maybe this CL is trying to solve a non-existing problem, as separate issues may
be tracked as bugs, fixed, and unittests added for each of them as appropriate,
as happens in https://reviews.llvm.org/D30487, which adds an option to break
multiple inheritance d
sylvestre.ledru added a comment.
Manuel, is that ok with you? thanks (I will rename the file)
https://reviews.llvm.org/D30111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sylvestre.ledru added a comment.
I will rename it before the upload if that is fine with you.
Comment at: unittests/Format/check-coding-style-mozilla.cpp:77
+return mVar;
+ } // Tiny functions can be written in a single line.
+
krasimir wrote:
> Note that
sylvestre.ledru updated this revision to Diff 90053.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D30111
Files:
test/Format/check-coding-style-mozilla.cpp
unittests/Format/CMakeLists.txt
Index: unittests/Format/CMakeLists.txt
===
krasimir added a comment.
In https://reviews.llvm.org/D30111#688379, @sylvestre.ledru wrote:
> @krasimir is that ok with you?
Thank you for addressing my comments! The descriptions help to see what's
supposed to happen.
Now another point with this test is that it tests that already Mozilla-va
sylvestre.ledru added a comment.
@krasimir is that ok with you?
https://reviews.llvm.org/D30111
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sylvestre.ledru updated this revision to Diff 89094.
sylvestre.ledru added a comment.
Sorry, I tried to rename the file but this is too confusing for Phabricator it
seems...
https://reviews.llvm.org/D30111
Files:
unittests/Format/check-coding-style-mozilla.cpp
Index: unittests/Format/check-
sylvestre.ledru added inline comments.
Comment at: test/Format/check-coding-style-mozilla.cpp:48
+,
+public Y
+{
krasimir wrote:
> sylvestre.ledru wrote:
> > krasimir wrote:
> > > This does not check precisely what the comment says
sylvestre.ledru updated this revision to Diff 89093.
sylvestre.ledru marked 3 inline comments as done.
https://reviews.llvm.org/D30111
Files:
unittests/Format/CheckCodingStyleMozilla.cpp
Index: unittests/Format/CheckCodingStyleMozilla.cpp
===
krasimir added inline comments.
Comment at: test/Format/check-coding-style-mozilla.cpp:10
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {
sylvestre.ledru wrote:
> krasimir wrote:
> > What is tested here? Brace styles?
> Yes, do you
klimek added inline comments.
Comment at: test/Format/check-coding-style-mozilla.cpp:7-9
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.
sylvestre.ledru marked 3 inline comments as done.
sylvestre.ledru added inline comments.
Comment at: test/Format/check-coding-style-mozilla.cpp:10
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {
krasimir wrote:
> What is tested here
sylvestre.ledru updated this revision to Diff 89022.
https://reviews.llvm.org/D30111
Files:
test/Format/check-coding-style-mozilla.cpp
Index: test/Format/check-coding-style-mozilla.cpp
===
--- test/Format/check-coding-style-mozill
krasimir added inline comments.
Comment at: test/Format/check-coding-style-mozilla.cpp:10
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+if (true) {
What is tested here? Brace styles?
Comment at: test/Format/check-coding-sty
sylvestre.ledru created this revision.
To avoid potential regressions and checking most of the coding style aspects at
once
https://reviews.llvm.org/D30111
Files:
test/Format/check-coding-style-mozilla.cpp
Index: test/Format/check-coding-style-mozilla.cpp
===
19 matches
Mail list logo
