ci: install rocksdb dependencies in CI #568
Conversation
WalkthroughThe pull request introduces modifications to two GitHub Actions workflow files: Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/rust.yml (1)
37-39: Consider adding dependency installation to other jobsThe new dependency installation step is currently only added to the Linting job. However, these dependencies (especially RocksDB) might be required for other jobs as well, particularly the Testing job.
To ensure consistency across the CI environment and prevent potential issues:
- Consider adding the dependency installation step to the Testing job.
- If the Formatting job doesn't require these dependencies, you can leave it as is.
This approach will help maintain a consistent environment across different CI stages and reduce the risk of environment-related failures.
.github/workflows/release-please.yml (1)
40-42: Enhance dependency installation stepThe addition of this step aligns well with the PR objective of installing RocksDB dependencies in CI. However, consider the following improvements:
Make this step conditional on release creation, similar to other steps in this job:
- name: Install dependencies if: ${{ needs.release-please.outputs.release_created }} run: sudo apt-get update && sudo apt-get install -y librocksdb-dev libsqlite3-devConsider pinning package versions to ensure build consistency:
run: sudo apt-get update && sudo apt-get install -y librocksdb-dev=<version> libsqlite3-dev=<version>If running in a container or with root privileges, you can omit
sudo. Otherwise, addsudoto ensure the command has the necessary permissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/release-please.yml (1 hunks)
- .github/workflows/rust.yml (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.github/workflows/rust.yml (1)
Line range hint
1-39: Overall approval of changesThe addition of the dependency installation step is a positive improvement to the CI workflow. It addresses the PR objective of installing RocksDB dependencies in CI.
The changes are well-placed and contribute to a more robust CI process. With the suggested improvements implemented, this change will enhance the reliability and consistency of the build environment.
🧰 Tools
🪛 actionlint
41-41: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/release-please.yml (1)
Line range hint
1-101: Overall workflow improvementThe addition of the dependency installation step enhances the CI process by ensuring that necessary libraries (RocksDB and SQLite) are available for the build. This change aligns well with the PR objective of installing RocksDB dependencies in CI.
The placement of the new step before the toolchain installation is appropriate. However, consider implementing the suggestions in the previous comment to further improve the workflow's efficiency and consistency.
🧰 Tools
🪛 actionlint
44-44: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/rust.yml
Outdated
| - name: Install dependencies | ||
| run: apt-get update && apt-get install -y librocksdb-dev libsqlite3-dev |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance dependency installation step
The new step for installing dependencies is a good addition. However, consider the following improvements:
- Use
sudoto ensure the command has the necessary permissions. - Add the
-yflag toapt-get updateto automatically answer yes to prompts. - Use
--no-install-recommendsto minimize installed packages. - Add error handling and verbosity for better debugging.
Here's an improved version of the step:
- name: Install dependencies
run: |
sudo apt-get update -y
sudo apt-get install -y --no-install-recommends librocksdb-dev libsqlite3-dev
shell: bashThis change ensures proper permissions, reduces unnecessary package installations, and provides a more robust command execution.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #568 +/- ##
==========================================
+ Coverage 73.99% 74.01% +0.02%
==========================================
Files 26 26
Lines 9849 9857 +8
==========================================
+ Hits 7288 7296 +8
Misses 2561 2561 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit