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 <lyn...@fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: osmith <osm...@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 <lafo...@osmocom.org> Comment-In-Reply-To: lynxis lazus <lyn...@fe80.eu> Gerrit-MessageType: comment
