-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update all of the plugins and verify in CI that they've stayed up-to-date. #435
Conversation
WalkthroughThe pull request introduces a new step in the GitHub Actions workflow for a Rust project, specifically within the Changes
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
DryRun Security SummaryThe GitHub Actions workflow has been updated to include a script that updates test plugin dependencies, ensuring the latest versions are used and potentially improving the project's security. Expand for full summarySummary: The provided code changes focus on updating the dependencies in the Rust project's test plugins. The key changes include:
Files Changed:
Code AnalysisWe ran |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/rust.yml (1)
149-150
: Consider moving the update step before the test executionThe plugin update step is currently placed after all tests have run. This means we're potentially testing against outdated dependencies. Consider moving this step before the "Run bulwark tests" step to ensure we're testing the latest versions.
Suggested workflow order:
- Checkout and setup
- Update plugins
- Run tests
- Verify clean working directory
This ensures that:
- Tests run against the latest dependencies
- Any compatibility issues are caught early
- The dirty repo check verifies both updates and tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
tests/plugins/multi-phase-plugin-a/Cargo.lock
is excluded by!**/*.lock
tests/plugins/multi-phase-plugin-b/Cargo.lock
is excluded by!**/*.lock
tests/plugins/redis-plugin/Cargo.lock
is excluded by!**/*.lock
tests/plugins/smoke-test/Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
.github/workflows/rust.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/rust.yml (1)
149-150
: Add version verification for shared dependencies
To fully meet the PR objective of preventing missed updates, consider adding verification that shared dependencies are at the same version across plugins.
Add this verification script:
This helps identify any dependencies that might have different versions across plugins, ensuring consistency.
Consider adding a new step after the update:
- name: Verify plugin dependency consistency
run: |
#!/bin/bash
set -euo pipefail
echo "Checking for dependency version mismatches..."
MISMATCHES=$(find tests/plugins -name "Cargo.lock" -exec cargo metadata --format-version 1 --manifest-path {} \; | \
jq -r '.packages[] | select(.source != null) | "\(.name)@\(.version)"' | \
sort | uniq -c | \
awk '{if($1>1) print}' | \
grep -v "^[[:space:]]*1[[:space:]]" || true)
if [ ! -z "$MISMATCHES" ]; then
echo "::warning::Found dependency version mismatches:"
echo "$MISMATCHES"
fi
More verbose but a lot more readable. Good change. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/rust.yml (1)
153-153
: Minor: Remove trailing spacesThere are trailing spaces on lines 153 and 160.
- + - +Also applies to: 160-160
🧰 Tools
🪛 yamllint (1.35.1)
[error] 153-153: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/rust.yml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/rust.yml
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/rust.yml (1)
149-171
: Implementation looks great!
The code follows best practices with:
- Proper error handling using
set -euo pipefail
- Maintainable plugin list using an array
- Safe directory navigation using subshells
- Clear error reporting using GitHub Actions error logging
🧰 Tools
🪛 yamllint (1.35.1)
[error] 153-153: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
The fix from #434 didn't get applied to the plugins. This addresses that and prevents similar issues in the future.
Summary by CodeRabbit