Change in ...coreboot[master]: util/lint: check correctness of Change-Id line

[Idwer Vollering (Code Review)]

Idwer Vollering has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33884

Change subject: util/lint: check correctness of Change-Id line
......................................................................

util/lint: check correctness of Change-Id line

Change-Id: I67b9f134500bb596ae5790b68fe9f27e2fa2cfb4
Signed-off-by: Idwer Vollering 
---
A util/lint/lint-extended-024-change-id
1 file changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/33884/1

diff --git a/util/lint/lint-extended-024-change-id b/util/lint/lint-extended-024-change-id
new file mode 100755
index 0000000..1989ed4
--- /dev/null
+++ b/util/lint/lint-extended-024-change-id
@@ -0,0 +1,10 @@
+#!/bin/sh
+if [ "$(git log -n 1 | grep -c '[[:space:]]+Change-Id: ')" -lt 1 ]
+then
+	echo "Change-Id line is missing";
+fi
+
+if [ "$(git log -n 1 | grep -c '[[:space:]]+Change-Id: ')" -gt 1 ]
+then
+	echo "Found more than one Change-Id line";
+fi

Change in ...coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Idwer Vollering (Code Review)]

Hello Paul Menzel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33884

to look at the new patch set (#2).

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

util/lint: check for Change-Id line presence and duplicates

Change-Id: I67b9f134500bb596ae5790b68fe9f27e2fa2cfb4
Signed-off-by: Idwer Vollering 
---
A util/lint/lint-extended-024-change-id
1 file changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/33884/2

Change in ...coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Idwer Vollering (Code Review)]

Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 2:

I wrote this to fail tests, see https://review.coreboot.org/c/coreboot/+/30987
Should it end up in checkpatch.pl?

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 2:
> Patch Set 2:
>
> I wrote this to fail tests, see https://review.coreboot.org/c/coreboot/+/30987
> Should it end up in checkpatch.pl?

You mean to fail tests on jenkins? That should work since what-jenkins-does calls lint with --junit, which _should_ emit test results. I think something is broken there.

No need to touch checkpatch though.

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33884/2/util/lint/lint-extended-024... 
File util/lint/lint-extended-024-change-id:

https://review.coreboot.org/c/coreboot/+/33884/2/util/lint/lint-extended-024... 
PS2, Line 2: $(git log -n 1 | grep -c '[[:space:]]+Change-Id: ')
I guess it doesn't matter time-wise, but why not store the result in a variable instead of executing twice?

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 2: Code-Review+1

Thanks for this patch!

Looks good, but haven't taken a proper look at it yet.

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 2:

Just to be sure, for developers having linting enabled, this will be run after the commit message has been written and updated by the “Change-Id adding hook”?

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Idwer Vollering (Code Review)]

Hello Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33884

to look at the new patch set (#3).

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

util/lint: check for Change-Id line presence and duplicates

Change-Id: I67b9f134500bb596ae5790b68fe9f27e2fa2cfb4
Signed-off-by: Idwer Vollering 
---
A util/lint/lint-extended-024-change-id
1 file changed, 12 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/33884/3

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Idwer Vollering (Code Review)]

Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33884/2/util/lint/lint-extended-024... 
File util/lint/lint-extended-024-change-id:

https://review.coreboot.org/c/coreboot/+/33884/2/util/lint/lint-extended-024... 
PS2, Line 2: $(git log -n 1 | grep -c '[[:space:]]+Change-Id: ')
> I guess it doesn't matter time-wise, but why not store the result in a variable instead of executing […]

Done

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33884/3/util/lint/lint-extended-024... 
File util/lint/lint-extended-024-change-id:

https://review.coreboot.org/c/coreboot/+/33884/3/util/lint/lint-extended-024... 
PS3, Line 2: CHANGE_ID_LINECOUNT=$(git log -n 1 | grep -c '[[:space:]]+Change-Id: ')
Original-Change-Id is not matched, right?

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3:

(1 comment)
> Just to be sure, for developers having linting enabled, this will be run after the commit message has been written and updated by the “Change-Id adding hook”?

I was also wondering when it is supposed to be run?

https://review.coreboot.org/c/coreboot/+/33884/3/util/lint/lint-extended-024... 
File util/lint/lint-extended-024-change-id:

https://review.coreboot.org/c/coreboot/+/33884/3/util/lint/lint-extended-024... 
PS3, Line 2: CHANGE_ID_LINECOUNT=$(git log -n 1 | grep -c '[[:space:]]+Change-Id: ')
> Original-Change-Id is not matched, right?

Right.

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3: -Code-Review

I double checked, and our lint scripts are executed in the pre-commit hook (but only the "stable" ones), which is run before the commit-msg hook that adds Change-Id.

So what's the intended behavior for this?

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Idwer Vollering (Code Review)]

Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3:
> Patch Set 3: -Code-Review
>
> I double checked, and our lint scripts are executed in the pre-commit hook (but only the "stable" ones), which is run before the commit-msg hook that adds Change-Id.
>
> So what's the intended behavior for this?

Meaning the file should be renamed (to lint-stable-024-change-id), in order to be run from jenkins?

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33884 )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Patch Set 3: Code-Review+1
> Patch Set 3:
> > Patch Set 3: -Code-Review
> >
> > I double checked, and our lint scripts are executed in the pre-commit hook (but only the "stable" ones), which is run before the commit-msg hook that adds Change-Id.
> >
> > So what's the intended behavior for this?
>
> Meaning the file should be renamed (to lint-stable-024-change-id), in order to be run from jenkins?

No, jenkins runs both stable and extended (see util/testing/Makefile.inc:93) while pre-commit only runs stable (where this would break: Change-Id isn't added yet). So this is fine.

Please state the intent that this is for Jenkins' sanity tests in the commit message, because it appears to me that folks were wondering how this affects "git commit".

Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Idwer Vollering (Code Review)]

Hello HAOUAS Elyes, Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33884

to look at the new patch set (#4).

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

util/lint: check for Change-Id line presence and duplicates

This check should run during pre-commit time (before pushing).

Change-Id: I67b9f134500bb596ae5790b68fe9f27e2fa2cfb4
Signed-off-by: Idwer Vollering 
---
A util/lint/lint-stable-024-change-id
1 file changed, 12 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/33884/4

[S] Change in coreboot[master]: util/lint: check for Change-Id line presence and duplicates

[Martin L Roth (Code Review)]

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33884?usp=email )

Change subject: util/lint: check for Change-Id line presence and duplicates
......................................................................

Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.