refactor: replace reviewdog with direct cargo calls and PR comment by alexmohr · Pull Request #67 · eclipse-opensovd/cicd-workflows

alexmohr · 2026-06-16T15:06:12Z

Summary

Remove rust-lint-and-format-action reviewdog integration (giraffate/clippy-action, reviewdog setup, checkstyle XML helpers)
Remove shared-config/cargo-fmt.sh and cargo-clippy.sh; pre-commit hooks now call cargo directly (language: system)
Nightly clippy runs warn-only (continue-on-error); findings are posted as a PR comment with a disclaimer that they are informational and may be unrelated to PR changes; no comment posted when clean
New action inputs: toolchain, all-features, post-pr-comment
Document RUSTUP_TOOLCHAIN=nightly as the local opt-in for nightly clippy via prek

Checklist

I have tested my changes locally
I have added or updated documentation
I have linked related issues or discussions
I have added or updated tests

Notes for Reviewers

floroks · 2026-06-29T09:06:16Z

+      run: |
+        git fetch --depth=1 origin "${{ inputs.base-sha }}" 2>/dev/null || true
+        git diff "${{ inputs.base-sha }}...HEAD" --unified=0 > /tmp/pr.diff
+        echo "diff_file=/tmp/pr.diff" >> "$GITHUB_OUTPUT"


question: Why is it put as output result, but never used?

floroks · 2026-06-29T09:21:07Z

-      uses: giraffate/clippy-action@v1
+    - name: Post nightly-clippy comment on PR
+      if: >-
+        inputs.post-pr-comment == 'true' && github.event_name == 'pull_request' && steps.clippy.outputs.has_findings == 'true' && github.event.pull_request.head.repo.full_name == github.repository


I think this is missing the ${{ }}, otherwise it's not evaluated

Seems to work fine on the test PR I did on CDA eclipse-opensovd/classic-diagnostic-adapter#385 (comment), anyhow I will test again once I addressed the remaining issues :)

No it's not necessary. But after looking at this again after some passed since I created this PR I was still pretty unhappy with how complicated it still is. I dropped all the comment handling for format and clippy, just look at the pipeline output directly.
I did add two small wrapper scripts, so we can configure pre commit properly for format and clippy.

Example for CDA.
Now if you run pre-commit it will run format with the correct nightly stage and clippy with 1.88 and the pinned nightly version.
We might want to update the nightly version to something more recent in the future but that is out of scope for here.

- id: cargo-fmt args: - --toolchain=nightly-2025-07-14 - --all - id: cargo-clippy name: clippy (stable) args: - --toolchain=1.88 - --all-targets - --locked - -- - -D - warnings - id: cargo-clippy name: clippy (nightly) args: - --toolchain=nightly-2025-07-14 - --all-targets - --locked - -- - -D - warnings

floroks · 2026-07-13T05:54:37Z

needs rebase (conflict in README)

- Remove rust-lint-and-format-action reviewdog integration (giraffate/clippy-action, reviewdog setup, checkstyle XML helpers) - Remove shared-config/cargo-fmt.sh and cargo-clippy.sh; pre-commit hooks now call cargo directly (language: system) - Nightly clippy runs warn-only (continue-on-error); findings are posted as a PR comment with a disclaimer that they are informational and may be unrelated to PR changes; no comment posted when clean - New action inputs: toolchain, all-features, post-pr-comment - Document RUSTUP_TOOLCHAIN=nightly as the local opt-in for nightly clippy via prek Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>

alexmohr · 2026-07-23T18:48:10Z

@floroks rebased :)

mbfm

Mostly nitpicks, but I spotted one bug as well.

mbfm · 2026-08-28T06:54:40Z

+    cargo = "cargo"
+    cmd = [cargo]


Nitpick:

Suggested change

cargo = "cargo"

cmd = [cargo]

cmd = ["cargo"]

I am going to assume you also wanted me to remove the pointless variable above :)

mbfm · 2026-08-28T07:13:10Z

+    cargo = "cargo"
+    cmd = [cargo]


Nitpick:

Suggested change

cargo = "cargo"

cmd = [cargo]

cmd = ["cargo"]

mbfm · 2026-08-28T07:17:00Z

-    description: 'Reporter to use for reviewdog'
-    required: false
-    default: 'github-pr-review'
+    default: '--all-features --all-targets --D warnings'


I think, this should be:

Suggested change

default: '--all-features --all-targets --D warnings'

default: '--all-features --all-targets --locked -- --deny=warnings'

The --locked is from .pre-commit-hooks.yaml. Not sure, if you maybe decided against that here.

Makes sense, done

mbfm · 2026-08-28T07:25:17Z

-          || FMT_EXIT=$?
+    - name: Format check (rustfmt)
+      id: fmt
+      continue-on-error: true


Suggested change

continue-on-error: true

continue-on-error: true # fail later to present all errors to user at once

I think, you mentioned this to me at some point. Maybe good to document that. 🙂

Using same comment for both lines.

mbfm · 2026-08-28T07:25:51Z

-          /tmp/checkstyle.xml /tmp/rustfmt_stderr.log "$STDERR_SEVERITY"
+    - name: Clippy
+      id: clippy
+      continue-on-error: true


Suggested change

continue-on-error: true

continue-on-error: true # fail later to present all errors to user at once

Using # Set to 'true' to present all errors to user at once, 'false' fails immediately.

mbfm · 2026-08-28T07:29:03Z

+    - -D
+    - warnings


Easier to understand like this:

Suggested change

- -D

- warnings

- --deny=warnings

thanks, done :)

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>

alexmohr force-pushed the feature/improve-workflows branch 12 times, most recently from 3488859 to bc47422 Compare June 17, 2026 12:47

alexmohr marked this pull request as ready for review June 17, 2026 13:37

alexmohr force-pushed the feature/improve-workflows branch 2 times, most recently from 0ae1c3c to 5afa22d Compare June 18, 2026 20:00

floroks reviewed Jun 29, 2026

View reviewed changes

Comment thread .pre-commit-hooks.yaml Outdated

floroks reviewed Jun 29, 2026

View reviewed changes

alexmohr force-pushed the feature/improve-workflows branch 12 times, most recently from 780f272 to c512250 Compare July 1, 2026 09:14

alexmohr added 3 commits July 23, 2026 20:35

fix(review): use diff file variable

334266e

fix(comments): make sure that comments are posted)

0f19d76

alexmohr force-pushed the feature/improve-workflows branch from c512250 to 0f19d76 Compare July 23, 2026 18:35

fix(clippy): install clippy too

f5696ba

floroks previously approved these changes Aug 27, 2026

View reviewed changes

mbfm suggested changes Aug 28, 2026

View reviewed changes

alexmohr dismissed floroks’s stale review via 14e4fcd August 28, 2026 10:16

alexmohr force-pushed the feature/improve-workflows branch from 14e4fcd to 6708173 Compare August 28, 2026 10:36

fix(review-findings): all review findings applied

dc0f14e

Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>

alexmohr force-pushed the feature/improve-workflows branch from 6708173 to dc0f14e Compare August 28, 2026 11:07

mbfm approved these changes Aug 28, 2026

View reviewed changes

floroks approved these changes Aug 31, 2026

View reviewed changes

alexmohr merged commit febc566 into main Aug 31, 2026
4 checks passed

alexmohr deleted the feature/improve-workflows branch August 31, 2026 08:02

	default: '--all-features --all-targets --D warnings'
	default: '--all-features --all-targets --locked -- --deny=warnings'

	continue-on-error: true
	continue-on-error: true # fail later to present all errors to user at once

Conversation

alexmohr commented Jun 16, 2026

Summary

Checklist

Related

Notes for Reviewers

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

floroks Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

floroks commented Jul 13, 2026

Uh oh!

alexmohr commented Jul 23, 2026

Uh oh!

mbfm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

floroks Jun 29, 2026 •

edited

Loading