ci(verify-nix): Only build changed plugins #40

Open
martin wants to merge 1 commit from refactor/verify-nix into main
Owner
No description provided.
martin self-assigned this 2026-08-23 13:14:00 +00:00
ci(verify-nix): Only build changed plugins
Some checks failed
ci/woodpecker/pr/verify-nix Pipeline failed
ci/woodpecker/pr/pr-review Pipeline was successful
bc53c71199
Member

Review Summary

The PR introduces a valuable optimization to only build changed Nix plugins instead of all plugins on every run. The approach is sound: detect changed files via CI_PIPELINE_FILES, classify changes as either shared infrastructure (triggering full build) or plugin-specific (building only affected plugins).

However, there's a critical bug in the shell script where $$ is used instead of $ for parameter expansion (lines 70-71). This will cause plugin name extraction to fail completely, as $$ expands to the shell's PID rather than performing the intended string manipulation.

Other observations:

  • The expanded trigger paths are comprehensive and correct
  • Conservative fallback to full build on parse errors is appropriate
  • Cachix push error handling is reasonable for CI
  • Script complexity is high for inline YAML; consider extracting to a separate script file for maintainability

*🤖 Review by opencode (opencode/nemotron-3-ultra-free) — verdict: changes

# Review Summary The PR introduces a valuable optimization to only build changed Nix plugins instead of all plugins on every run. The approach is sound: detect changed files via `CI_PIPELINE_FILES`, classify changes as either shared infrastructure (triggering full build) or plugin-specific (building only affected plugins). However, there's a **critical bug** in the shell script where `$$` is used instead of `$` for parameter expansion (lines 70-71). This will cause plugin name extraction to fail completely, as `$$` expands to the shell's PID rather than performing the intended string manipulation. Other observations: - The expanded trigger paths are comprehensive and correct - Conservative fallback to full build on parse errors is appropriate - Cachix push error handling is reasonable for CI - Script complexity is high for inline YAML; consider extracting to a separate script file for maintainability --- *🤖 Review by opencode (opencode/nemotron-3-ultra-free) — verdict: **changes** <!-- forgejo-opencode-review -->
clank-bot left a comment

Code review findings from opencode.

Code review findings from opencode. <!-- forgejo-opencode-review -->
@ -32,0 +67,4 @@
case " $plugins " in
*" $plugin "*) ;;
*) plugins="$plugins $plugin" ;;
esac
Member
[bug] Using $$ for parameter expansion is incorrect. In shell, $$ expands to the process ID. Should be ${path#plugins/} (single $). This bug will cause plugin name extraction to fail.
<!-- forgejo-opencode-review --> [bug] Using $$ for parameter expansion is incorrect. In shell, $$ expands to the process ID. Should be ${path#plugins/} (single $). This bug will cause plugin name extraction to fail.
Author
Owner

This is not correct, in woodpecker ${} will be replaced by the woodpecker preprocessor, while $${} must be used for shell variables

This is not correct, in woodpecker ${} will be replaced by the woodpecker preprocessor, while $${} must be used for shell variables
@ -32,0 +68,4 @@
*" $plugin "*) ;;
*) plugins="$plugins $plugin" ;;
esac
;;
Member
[bug] Same bug: $$ should be $ for parameter expansion. Should be ${plugin%%/*}
<!-- forgejo-opencode-review --> [bug] Same bug: $$ should be $ for parameter expansion. Should be ${plugin%%/*}
Some checks failed
ci/woodpecker/pr/verify-nix Pipeline failed
ci/woodpecker/pr/pr-review Pipeline was successful
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin refactor/verify-nix:refactor/verify-nix
git switch refactor/verify-nix

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff refactor/verify-nix
git switch refactor/verify-nix
git rebase main
git switch main
git merge --ff-only refactor/verify-nix
git switch refactor/verify-nix
git rebase main
git switch main
git merge --no-ff refactor/verify-nix
git switch main
git merge --squash refactor/verify-nix
git switch main
git merge --ff-only refactor/verify-nix
git switch main
git merge refactor/verify-nix
git push origin main
Sign in to join this conversation.
No description provided.