-
Notifications
You must be signed in to change notification settings - Fork 3
Update CI workflows for consistency and performance #19
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
Conversation
Update CI workflows to improve performance and consistency. * **CI Workflow (`.github/workflows/ci.yml`)** - Update `actions-rs/toolchain@v1` to `dtolnay/rust-toolchain@stable` - Update `actions-rs/cargo@v1` to `dtolnay/rust-toolchain@stable` with direct `run` commands - Update `actions/checkout@v3` to `actions/checkout@v4` - Update `actions/cache@v3` to `actions/cache@v4` - Fix code coverage step to use a working version of `cargo-tarpaulin` - Add explicit working directory for all cargo commands - Update `codecov/codecov-action@v3` to `codecov/codecov-action@v4` * **Benchmarks Workflow (`.github/workflows/benchmarks.yml`)** - Update `actions/checkout@v3` to `actions/checkout@v4` - Update `actions-rs/toolchain@v1` to `dtolnay/rust-toolchain@stable` - Update `actions-rs/cargo@v1` to `dtolnay/rust-toolchain@stable` with direct `run` commands - Update `actions/cache@v3` to `actions/cache@v4` - Fix `actions-rs/[email protected]` step to use a direct cargo install command - Add explicit working directory for all cargo commands - Fix benchmark comparison step * **Cross-Platform Workflow (`.github/workflows/cross-platform.yml`)** - Update `actions/checkout@v3` to `actions/checkout@v4` - Update `actions-rs/toolchain@v1` to `dtolnay/rust-toolchain@stable` - Update `actions-rs/cargo@v1` to `dtolnay/rust-toolchain@stable` with direct `run` commands - Update `actions/cache@v3` to `actions/cache@v4` - Add explicit working directory for all cargo commands * **Release Workflow (`.github/workflows/release.yml`)** - Update `actions/checkout@v3` to `actions/checkout@v4` - Update `actions-rs/toolchain@v1` to `dtolnay/rust-toolchain@stable` - Update `actions-rs/cargo@v1` to `dtolnay/rust-toolchain@stable` with direct `run` commands - Update `actions/upload-artifact@v3` to `actions/upload-artifact@v4` - Update `actions/download-artifact@v3` to `actions/download-artifact@v4` - Update `peaceiris/actions-gh-pages@v3` to the latest version - Add explicit working directory for all cargo commands * **Security Workflow (`.github/workflows/security.yml`)** - Update `actions/checkout@v3` to `actions/checkout@v4` - Update `actions-rs/toolchain@v1` to `dtolnay/rust-toolchain@stable` - Update `actions-rs/cargo@v1` to `dtolnay/rust-toolchain@stable` with direct `run` commands - Fix `actions-rs/[email protected]` step to use a direct cargo install command - Update `github/codeql-action/init@v2` to `github/codeql-action/init@v3` - Update `github/codeql-action/analyze@v2` to `github/codeql-action/analyze@v3` - Update `actions/dependency-review-action@v3` to `actions/dependency-review-action@v4` - Add explicit working directory for all cargo commands --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/openSVM/osvm-cli?shareId=XXXX-XXXX-XXXX-XXXX).
|
Unable to perform a code review. You have run out of credits 😔 |
Reviewer's Guide by SourceryThis pull request updates the CI workflows to improve consistency and performance. It migrates from No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @0xrinegade - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a matrix strategy to avoid repeating the same steps across multiple workflows.
- The consistency improvements across workflows look good.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| crate: cargo-tarpaulin | ||
| version: latest | ||
| use-tool-cache: true | ||
| run: cargo install cargo-tarpaulin |
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.
suggestion (performance): Consider caching the installation for cargo-tarpaulin.
Replacing the actions-based install with a direct cargo command improves simplicity, but installing cargo-tarpaulin every run could impact performance. Evaluate if caching the installed binary is feasible to reduce CI time.
Suggested implementation:
- name: Cache cargo-tarpaulin
id: cache-cargo-tarpaulin
uses: actions/cache@v2
with:
path: ~/.cargo/bin/cargo-tarpaulin
key: ${{ runner.os }}-cargo-tarpaulin
- name: Install cargo-tarpaulin
if: steps.cache-cargo-tarpaulin.outputs.cache-hit != 'true'
run: cargo install cargo-tarpaulin
working-directory: .
Ensure that the cache key is updated if you ever change the version of cargo-tarpaulin being installed,
so that a new version is installed when needed.
Also, verify the order of steps in your workflow so that the caching step runs before the installation.
.github/workflows/benchmarks.yml
Outdated
| crate: cargo-criterion | ||
| version: latest | ||
| use-tool-cache: true | ||
| run: cargo install cargo-criterion |
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.
suggestion (performance): Evaluate caching for cargo-criterion installation.
Similar to other installation steps, running cargo install for cargo-criterion on each workflow execution may slow down builds. Consider whether caching the result of this installation could lead to overall performance improvements.
Suggested implementation:
- name: Cache dependencies
- name: Cache cargo-criterion
id: cache-cargo-criterion
uses: actions/cache@v2
with:
path: ~/.cargo/bin/cargo-criterion
key: ${{ runner.os }}-cargo-criterion-${{ hashFiles('**/Cargo.lock') }}
- name: Install cargo-criterion
if: steps.cache-cargo-criterion.outputs.cache-hit != 'true'
run: cargo install cargo-criterion
working-directory: .
Ensure that the cache key is appropriate for your project. If you later add custom flags or the version of cargo-criterion changes, update the key to force cache refresh.
Update CI workflows to improve performance and consistency.
CI Workflow (
.github/workflows/ci.yml)actions-rs/toolchain@v1todtolnay/rust-toolchain@stableactions-rs/cargo@v1todtolnay/rust-toolchain@stablewith directruncommandsactions/checkout@v3toactions/checkout@v4actions/cache@v3toactions/cache@v4cargo-tarpaulincodecov/codecov-action@v3tocodecov/codecov-action@v4Benchmarks Workflow (
.github/workflows/benchmarks.yml)actions/checkout@v3toactions/checkout@v4actions-rs/toolchain@v1todtolnay/rust-toolchain@stableactions-rs/cargo@v1todtolnay/rust-toolchain@stablewith directruncommandsactions/cache@v3toactions/cache@v4actions-rs/[email protected]step to use a direct cargo install commandCross-Platform Workflow (
.github/workflows/cross-platform.yml)actions/checkout@v3toactions/checkout@v4actions-rs/toolchain@v1todtolnay/rust-toolchain@stableactions-rs/cargo@v1todtolnay/rust-toolchain@stablewith directruncommandsactions/cache@v3toactions/cache@v4Release Workflow (
.github/workflows/release.yml)actions/checkout@v3toactions/checkout@v4actions-rs/toolchain@v1todtolnay/rust-toolchain@stableactions-rs/cargo@v1todtolnay/rust-toolchain@stablewith directruncommandsactions/upload-artifact@v3toactions/upload-artifact@v4actions/download-artifact@v3toactions/download-artifact@v4peaceiris/actions-gh-pages@v3to the latest versionSecurity Workflow (
.github/workflows/security.yml)actions/checkout@v3toactions/checkout@v4actions-rs/toolchain@v1todtolnay/rust-toolchain@stableactions-rs/cargo@v1todtolnay/rust-toolchain@stablewith directruncommandsactions-rs/[email protected]step to use a direct cargo install commandgithub/codeql-action/init@v2togithub/codeql-action/init@v3github/codeql-action/analyze@v2togithub/codeql-action/analyze@v3actions/dependency-review-action@v3toactions/dependency-review-action@v4For more details, open the Copilot Workspace session.
Summary by Sourcery
Updates CI workflows to improve consistency and performance by updating actions, fixing code coverage, and adding explicit working directories for cargo commands.
CI:
cargo-tarpaulin.