Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31916
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Revert "lint/clang-format: set to 96 chars per line"
This reverts commit 626ba097a2cd1c87800a2154420829b09803467e.
This change was submitted under the incorrect assumption that there was agreement on a coding style change. There wasn't, so while the issue is under discussion we should revert to the previous status quo.
Making clang-format honor the line length is a separate issue from changing the line length, and can be reuploaded as a separate CL.
Change-Id: I433c82c95a897b3113cace3668cc8ce0f1ab75bf --- M .clang-format M util/lint/lint-007-checkpatch 2 files changed, 3 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/31916/1
diff --git a/.clang-format b/.clang-format index 5c8aa3c..d853f50 100644 --- a/.clang-format +++ b/.clang-format @@ -7,7 +7,7 @@ IndentCaseLabels: false SortIncludes: false ContinuationIndentWidth: 8 -ColumnLimit: 96 +ColumnLimit: 0 AlwaysBreakBeforeMultilineStrings: true AllowShortLoopsOnASingleLine: false AllowShortFunctionsOnASingleLine: false diff --git a/util/lint/lint-007-checkpatch b/util/lint/lint-007-checkpatch index a7b63e8..afa593e 100755 --- a/util/lint/lint-007-checkpatch +++ b/util/lint/lint-007-checkpatch @@ -28,8 +28,6 @@ ^src/vendorcode|\ ^Documentation"
-opts="--max-line-length 96" - # default: test src and util if [ "$1" = "" ]; then INCLUDED_DIRS="src util" @@ -37,7 +35,7 @@ elif [ "$1" = "diff" ]; then args=$( echo $EXCLUDED_DIRS | \ sed -e 's,\|, ,g' -e 's,^,--exclude=,g' ) - util/lint/checkpatch.pl --quiet --no-signoff $opts $args - + util/lint/checkpatch.pl --quiet --no-signoff $args - exit $? # Space separated list of directories to test else @@ -51,5 +49,5 @@ grep -v $EXCLUDED_DIRS )
for FILE in $FILELIST; do - util/lint/checkpatch.pl --show-types --file --quiet $opts "$FILE" + util/lint/checkpatch.pl --show-types --file --quiet "$FILE" done
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31916
to look at the new patch set (#2).
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Revert "lint/clang-format: set to 96 chars per line"
This reverts commit 626ba097a2cd1c87800a2154420829b09803467e.
This change was submitted under the incorrect assumption that there was agreement on a coding style change. There wasn't, so while the issue is under discussion we should revert to the previous status quo.
Making clang-format honor the line length is a separate issue from changing the line length, and can be reuploaded as a separate CL.
Change-Id: I433c82c95a897b3113cace3668cc8ce0f1ab75bf Signed-off-by: Julius Werner jwerner@chromium.org --- M .clang-format M util/lint/lint-007-checkpatch 2 files changed, 3 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/31916/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31916 )
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Patch Set 2: Code-Review+2
Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31916 )
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Revert "lint/clang-format: set to 96 chars per line"
This reverts commit 626ba097a2cd1c87800a2154420829b09803467e.
This change was submitted under the incorrect assumption that there was agreement on a coding style change. There wasn't, so while the issue is under discussion we should revert to the previous status quo.
Making clang-format honor the line length is a separate issue from changing the line length, and can be reuploaded as a separate CL.
Change-Id: I433c82c95a897b3113cace3668cc8ce0f1ab75bf Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/31916 Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M .clang-format M util/lint/lint-007-checkpatch 2 files changed, 3 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/.clang-format b/.clang-format index 5c8aa3c..d853f50 100644 --- a/.clang-format +++ b/.clang-format @@ -7,7 +7,7 @@ IndentCaseLabels: false SortIncludes: false ContinuationIndentWidth: 8 -ColumnLimit: 96 +ColumnLimit: 0 AlwaysBreakBeforeMultilineStrings: true AllowShortLoopsOnASingleLine: false AllowShortFunctionsOnASingleLine: false diff --git a/util/lint/lint-007-checkpatch b/util/lint/lint-007-checkpatch index a7b63e8..afa593e 100755 --- a/util/lint/lint-007-checkpatch +++ b/util/lint/lint-007-checkpatch @@ -28,8 +28,6 @@ ^src/vendorcode|\ ^Documentation"
-opts="--max-line-length 96" - # default: test src and util if [ "$1" = "" ]; then INCLUDED_DIRS="src util" @@ -37,7 +35,7 @@ elif [ "$1" = "diff" ]; then args=$( echo $EXCLUDED_DIRS | \ sed -e 's,\|, ,g' -e 's,^,--exclude=,g' ) - util/lint/checkpatch.pl --quiet --no-signoff $opts $args - + util/lint/checkpatch.pl --quiet --no-signoff $args - exit $? # Space separated list of directories to test else @@ -51,5 +49,5 @@ grep -v $EXCLUDED_DIRS )
for FILE in $FILELIST; do - util/lint/checkpatch.pl --show-types --file --quiet $opts "$FILE" + util/lint/checkpatch.pl --show-types --file --quiet "$FILE" done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31916 )
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31916 )
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Patch Set 3:
Isn't there a rule against +2ing your own patches? What about waiting for 24 hours to merge? Are you running the coreboot project now?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31916 )
Change subject: Revert "lint/clang-format: set to 96 chars per line" ......................................................................
Patch Set 3:
Isn't there a rule against +2ing your own patches? What about waiting for 24 hours to merge? Are you running the coreboot project now?
This is not a normal patch, I assume there can be exceptions for reverting bad changes (at least I've seen that done before).
Okay, maybe I should have waited for a +2. It was Friday afternoon and I wanted to avoid that this stays in for another weekend after we had already wasted way too much time arguing about it. I appreciate Nico approving it after the fact.
-
Julius Werner (Code Review) -
Martin Roth (Code Review)
-
Nico Huber (Code Review)