Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35176 )
Change subject: util/lint: make clang-format non-fatal ......................................................................
util/lint: make clang-format non-fatal
The current clang-format configuration is completely broken. It forces one to change the code style of patches before pushing them, only to find out that checkpatch now complains about it. This means newcomers get scared away, and developers only get angered and frustrated about it, and end up working around clang-format's requirements anyway.
For now, make clang-format's complaints non-fatal, reducing them to text noise. However, since clang-format is currently unusable, reverting it out would be preferred.
Change-Id: Iffa8934efa1c27c04e10545f66d8f9976e74c367 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/skylake/chip_fsp20.c M util/lint/check-style M util/lint/lint-stable-022-clang-format 3 files changed, 3 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/35176/1
diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index 064f71e..bfd9480 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -127,8 +127,7 @@ * Found the first enabled device in * a given dev number. */ - printk(BIOS_INFO, "PCI func %d was swapped" - " to func 0.\n", i); + printk(BIOS_INFO, "PCI func %d was swapped to func 0.\n", i); func0->path.pci.devfn = dev->path.pci.devfn; dev->path.pci.devfn = devfn0; break; diff --git a/util/lint/check-style b/util/lint/check-style index 2237ed6..a0679d8 100755 --- a/util/lint/check-style +++ b/util/lint/check-style @@ -140,4 +140,4 @@ printf "Aborting commit. Apply changes and commit again or skip checking with" printf " --no-verify (not recommended).\n"
-exit 1 +exit 0 diff --git a/util/lint/lint-stable-022-clang-format b/util/lint/lint-stable-022-clang-format index bd662e4..48f51a8 100755 --- a/util/lint/lint-stable-022-clang-format +++ b/util/lint/lint-stable-022-clang-format @@ -33,6 +33,6 @@ if [ "$(git diff --no-prefix HEAD~..HEAD -- $files_to_check | clang-format-diff)" != "" ]; then echo "Coding style mismatch. The following patch fixes it:" git diff --no-prefix HEAD~..HEAD -- $files_to_check | clang-format-diff - exit 1 + exit 0 fi fi
Hello Patrick Rudolph, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35176
to look at the new patch set (#2).
Change subject: util/lint: make clang-format non-fatal ......................................................................
util/lint: make clang-format non-fatal
The current clang-format configuration is completely broken. It forces one to change the code style of patches before pushing them, only to find out that checkpatch now complains about it. This means newcomers get scared away, and developers only get angered and frustrated about it, and end up working around clang-format's requirements anyway.
For now, make clang-format's complaints non-fatal, reducing them to text noise. However, since clang-format is currently unusable, reverting it out would be preferred.
Change-Id: Iffa8934efa1c27c04e10545f66d8f9976e74c367 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M util/lint/check-style M util/lint/lint-stable-022-clang-format 2 files changed, 7 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/35176/2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35176 )
Change subject: util/lint: make clang-format non-fatal ......................................................................
Patch Set 2: Code-Review+1
I remember this being very confusing when I tried to make my first patch, especially since no one else seemed to be following the suggested clang-format rules. Right now it generates so much noise that until the style guide becomes mandatory (eg. integrated into Jenkins) then I think it should just be disabled entirely.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35176 )
Change subject: util/lint: make clang-format non-fatal ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35176 )
Change subject: util/lint: make clang-format non-fatal ......................................................................
util/lint: make clang-format non-fatal
The current clang-format configuration is completely broken. It forces one to change the code style of patches before pushing them, only to find out that checkpatch now complains about it. This means newcomers get scared away, and developers only get angered and frustrated about it, and end up working around clang-format's requirements anyway.
For now, make clang-format's complaints non-fatal, reducing them to text noise. However, since clang-format is currently unusable, reverting it out would be preferred.
Change-Id: Iffa8934efa1c27c04e10545f66d8f9976e74c367 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35176 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jacob Garber jgarber1@ualberta.ca Reviewed-by: Patrick Georgi pgeorgi@google.com --- M util/lint/check-style M util/lint/lint-stable-022-clang-format 2 files changed, 7 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Jacob Garber: Looks good to me, but someone else must approve
diff --git a/util/lint/check-style b/util/lint/check-style index 2237ed6..f72d7b4 100755 --- a/util/lint/check-style +++ b/util/lint/check-style @@ -137,7 +137,10 @@
printf "\nYou can apply these changes with:\n git apply $patch\n" printf "(may need to be called from the root directory of your repository)\n" -printf "Aborting commit. Apply changes and commit again or skip checking with" -printf " --no-verify (not recommended).\n"
-exit 1 +# FIXME: clang-format is currently unusable, so don't abort the commit. +# printf "Aborting commit. Apply changes and commit again or skip checking with" +# printf " --no-verify (not recommended).\n" +# exit 1 + +exit 0 diff --git a/util/lint/lint-stable-022-clang-format b/util/lint/lint-stable-022-clang-format index bd662e4..48f51a8 100755 --- a/util/lint/lint-stable-022-clang-format +++ b/util/lint/lint-stable-022-clang-format @@ -33,6 +33,6 @@ if [ "$(git diff --no-prefix HEAD~..HEAD -- $files_to_check | clang-format-diff)" != "" ]; then echo "Coding style mismatch. The following patch fixes it:" git diff --no-prefix HEAD~..HEAD -- $files_to_check | clang-format-diff - exit 1 + exit 0 fi fi
-
Angel Pons (Code Review) -
Jacob Garber (Code Review)
-
Patrick Georgi (Code Review)