Change in osmo-ci[master]: RFC: lint: annotate lines in gerrit

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

osmith gerrit-no-reply at lists.osmocom.org
Mon Nov 29 09:00:35 UTC 2021


osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/26393 )

Change subject: RFC: lint: annotate lines in gerrit
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

Thanks, I think this is a good improvement.

https://gerrit.osmocom.org/c/osmo-ci/+/26393/1//COMMIT_MSG 
Commit Message:

https://gerrit.osmocom.org/c/osmo-ci/+/26393/1//COMMIT_MSG@13 
PS1, Line 13: The jenkins nodes needs to access the gerrit via ssh
> I would expect one can createa gerrit user that has permission only to provide review and thereby limit the potential damage.

Yes, I would recommend this even if we can ensure that only reviewed osmo-ci.git code gets ssh access.

> the gerrit-lint isn't a problem to give ssh access since it will only execute reviewed code from osmo-ci. So we could just add ssh-key to jenkins and enable ssh-agent in jenkins.

Currently there's an exception: when modifying osmo-ci.git, the linter code gets executed from the repository state submitted to gerrit:

https://git.osmocom.org/osmo-ci/tree/jobs/gerrit-lint.yml?id=cccd0cdd084a68b67d22a903540d4a699ac376db#n32

So that needs to be removed. Probably best to remove the "cmd" variable from https://git.osmocom.org/osmo-ci/tree/jobs/gerrit-lint.yml?id=cccd0cdd084a68b67d22a903540d4a699ac376db#n8 to avoid setting another cmd in the future that points to the repository modified in gerrit, without thinking about ssh access the script would get. I suggest we put cmd here instead https://git.osmocom.org/osmo-ci/tree/jobs/gerrit-lint.yml?id=cccd0cdd084a68b67d22a903540d4a699ac376db#n90 so it cannot be overridden per project.


https://gerrit.osmocom.org/c/osmo-ci/+/26393/1/lint/lint_diff.sh 
File lint/lint_diff.sh:

https://gerrit.osmocom.org/c/osmo-ci/+/26393/1/lint/lint_diff.sh@34 
PS1, Line 34: if ! git diff -U0 "$COMMIT" | "$SCRIPT_DIR/checkpatch/checkpatch_osmo.sh" > gerrit_report ; then
This condition doesn't need to check if errors were found or not, because above it does "exit 0" if there are no errors. But this should not run by default, only if sending comments to gerrit is explicitly enabled. So it does not run if used locally as git pre-commit hook.

I suggest changing this to:

  if [ -n "$GERRIT_COMMENT" ]; then
  	git diff -U0 "$COMMIT" | "$SCRIPT_DIR/checkpatch/checkpatch_osmo.sh" > gerrit_report

And in jobs/gerrit-lint.yml, add GERRIT_COMMENT=1 before calling lint_diff.sh here: https://git.osmocom.org/osmo-ci/tree/jobs/gerrit-lint.yml?id=cccd0cdd084a68b67d22a903540d4a699ac376db#n8



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/26393
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ci
Gerrit-Branch: master
Gerrit-Change-Id: I1a48ddb976e0f53bfc0552d0be11e42ba68d9e49
Gerrit-Change-Number: 26393
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Comment-Date: Mon, 29 Nov 2021 09:00:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Comment-In-Reply-To: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211129/37ddc2e9/attachment.htm>


More information about the gerrit-log mailing list