ci/cd: configure github actions #2
Conversation
WalkthroughThis pull request introduces comprehensive configuration files for a Rust project's development workflow. The changes include setting up GitHub Actions for Continuous Integration (CI) and Continuous Deployment (CD), configuring dependency management with Dependabot, establishing code ownership rules, and adding tools for changelog generation, code formatting, and typo checking. The configurations aim to streamline the development process, automate dependency updates, and maintain code quality. Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Repo as GitHub Repository
participant CI as GitHub Actions
participant Dependabot
participant Mergify
Dev->>Repo: Push changes
Repo->>CI: Trigger CI workflow
CI->>CI: Run checks, tests, lints
Dependabot->>Repo: Create dependency updates
Mergify->>Dependabot: Automatically merge updates
Possibly related issues
Poem
🪧 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 (
|
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (8)
release.sh (1)
21-22: Add safety measures to sed commandsThe current sed commands could be dangerous without backups:
-sed -E -i "s/^version = .* $msg$/version = \"${1#v}\" $msg/" zefiro*/Cargo.toml -sed -E -i "s/^version = .* $msg$/version = \"${1#v}\" $msg/" Cargo.toml +sed -E -i.bak "s/^version = .* $msg$/version = \"${1#v}\" $msg/" zefiro*/Cargo.toml +sed -E -i.bak "s/^version = .* $msg$/version = \"${1#v}\" $msg/" Cargo.toml.github/workflows/ci.yml (2)
3-13: LGTM! Consider restricting permissions furtherThe workflow triggers are well configured. However, consider following the principle of least privilege by explicitly setting read-only permissions for other scopes:
permissions: + contents: read pull-requests: write + security-events: write # For uploading code coverage
125-128: Enhance MSRV check script robustnessThe script could be improved for better error handling and reporting:
for package in zefiro-cli zefiro-core zefiro-ui; do - printf "Checking MSRV for %s..." "$package" - cargo msrv --output-format json --path "$package" verify | tail -n 1 | jq --exit-status '.success' + echo "Checking MSRV for $package..." + if ! output=$(cargo msrv --output-format json --path "$package" verify); then + echo "Failed to check MSRV for $package" + exit 1 + fi + if ! echo "$output" | tail -n 1 | jq --exit-status '.success' > /dev/null; then + echo "MSRV check failed for $package" + exit 1 + fi done +🧰 Tools
🪛 yamllint (1.35.1)
[error] 128-128: no new line character at the end of file
(new-line-at-end-of-file)
rustfmt.yml (2)
1-1: Consider updating to Rust 2021 editionThe project is using Rust 2018 edition. Consider updating to Rust 2021 edition to take advantage of newer language features.
-edition = "2018" +edition = "2021"
1-15: Add newline at end of fileAdd a newline at the end of the file to comply with POSIX standards.
struct_field_align_threshold = 20 wrap_comments = true +🧰 Tools
🪛 yamllint (1.35.1)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
.github/dependabot.yml (3)
4-8: Consider adjusting the Cargo update frequency and organization.
- Daily updates for Cargo dependencies might be too frequent and could create unnecessary noise. Consider using
weeklyinstead.- Similar to the GitHub Actions configuration, you might want to group Cargo updates (e.g., separate groups for major/minor/patch updates).
- package-ecosystem: cargo directory: "/" schedule: - interval: daily + interval: weekly open-pull-requests-limit: 10 + groups: + minor: + update-types: + - "minor" + patch: + update-types: + - "patch"
11-22: Good grouping strategy for GitHub Actions updates.The separation of minor and patch updates into groups is a good practice. However, consider explicitly defining how major version updates should be handled.
groups: minor: update-types: - "minor" patch: update-types: - "patch" + # Major updates ungrouped for individual review + major: + update-types: + - "major" + open-pull-requests-limit: 1🧰 Tools
🪛 yamllint (1.35.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
22-22: Add newline at end of file.Add a newline character at the end of the file to comply with POSIX standards.
- "patch" +🧰 Tools
🪛 yamllint (1.35.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/CODEOWNERS(1 hunks).github/dependabot.yml(1 hunks).github/mergify.yml(1 hunks).github/workflows/cd.yml(1 hunks).github/workflows/ci.yml(1 hunks)cliff.toml(1 hunks)release.sh(1 hunks)rustfmt.yml(1 hunks)typos.toml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/CODEOWNERS
- typos.toml
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/mergify.yml
[error] 7-7: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/cd.yml
[error] 50-50: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/ci.yml
[error] 128-128: no new line character at the end of file
(new-line-at-end-of-file)
.github/dependabot.yml
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
rustfmt.yml
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.4)
.github/workflows/cd.yml
16-16: 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)
25-25: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
43-43: 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/ci.yml
24-24: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
29-29: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
91-91: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
107-107: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Gitleaks (8.21.2)
release.sh
42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
64-65: Verify the necessity of skipping the git_upstream_remote testThe test
repo::test::git_upstream_remoteis being skipped in both test runs. This might indicate an underlying issue that should be addressed.Also applies to: 68-69
cliff.toml (1)
1-108: LGTM! Comprehensive changelog configurationThe configuration is well-structured with:
- Clear commit categorization
- Good use of conventional commits
- Smart filtering of noise (deps, PR commits)
- Proper issue linking
.github/dependabot.yml (1)
1-1: LGTM! Version 2 is correctly specified.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
.github/mergify.yml (2)
1-9: Add additional safety conditions for Dependabot mergesThe current configuration includes basic safety checks. Consider adding these additional conditions for enhanced security:
pull_request_rules: - name: Automatic merge for Dependabot pull requests conditions: - author=dependabot[bot] - check-success=test # Ensure CI passes - base=main # Only merge to main branch + - "#approved-reviews-by>=1" # Require at least one approval + - "-conflict" # Ensure no merge conflicts + - "-draft" # Ensure PR is not in draft actions: merge: method: squash🧰 Tools
🪛 yamllint (1.35.1)
[error] 9-9: no new line character at the end of file
(new-line-at-end-of-file)
9-9: Add newline at end of fileAdd a newline character at the end of the file to comply with POSIX standards.
method: squash +🧰 Tools
🪛 yamllint (1.35.1)
[error] 9-9: no new line character at the end of file
(new-line-at-end-of-file)
release.sh (1)
23-24: Improve version update robustnessThe version update logic could be more robust.
Add error checking:
+# Validate version format +if ! echo "${1#v}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$'; then + echo "Error: Invalid version format. Must follow semver." + exit 1 +fi + # update the version msg="# managed by release.sh" -sed -E -i "s/^version = .* $msg$/version = \"${1#v}\" $msg/" zefiro*/Cargo.toml -sed -E -i "s/^version = .* $msg$/version = \"${1#v}\" $msg/" Cargo.toml +for toml in zefiro*/Cargo.toml Cargo.toml; do + if [ ! -f "$toml" ]; then + echo "Warning: $toml not found" + continue + fi + sed -E -i "s/^version = .* $msg$/version = \"${1#v}\" $msg/" "$toml" +done.github/workflows/ci.yml (2)
14-27: Add cargo caching to improve CI performance.While the check job is well-structured, consider these improvements:
- Add cargo caching to speed up builds
- Consider if
--offlineis too restrictive for your CI needsAdd this before the cargo commands:
- name: Checkout uses: actions/checkout@v4 + - name: Cache cargo + uses: actions/cache@v3 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo-
57-63: Optimize test execution strategy.The test suite runs the same tests twice:
- Once with
cargo test- Again with
cargo tarpaulinSince tarpaulin already runs the tests while collecting coverage, the first test run is redundant.
- - name: Run tests - run: | - cargo test --all --no-default-features \ - -- --skip "repo::test::git_upstream_remote" - name: Run tests run: | cargo tarpaulin --out xml --verbose --all-features \ -- --skip "repo::test::git_upstream_remote"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/mergify.yml(1 hunks).github/workflows/cd.yml(1 hunks).github/workflows/ci.yml(1 hunks)release.sh(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/mergify.yml
[error] 9-9: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/ci.yml
[error] 116-116: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/cd.yml
[error] 28-28: syntax error: expected , but found ''
(syntax)
[warning] 33-33: wrong indentation: expected 5 but found 4
(indentation)
[error] 43-43: duplication of key "uses" in mapping
(key-duplicates)
[error] 47-47: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Gitleaks (8.21.2)
release.sh
44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 actionlint (1.7.4)
.github/workflows/cd.yml
8-8: could not parse as YAML: yaml: line 8: did not find expected key
(syntax-check)
🔇 Additional comments (6)
.github/workflows/cd.yml (1)
44-47: Uncomment and secure the publishing stepThe publishing step is currently commented out. Before uncommenting:
- Ensure the
CARGO_REGISTRY_TOKENsecret is set in repository settings- Verify the manifest path is correct
- Consider adding a dry-run step before actual publishing
Run this script to verify the manifest path:
🧰 Tools
🪛 yamllint (1.35.1)
[error] 47-47: no new line character at the end of file
(new-line-at-end-of-file)
release.sh (2)
39-39:⚠️ Potential issueAdd check for detailed.toml configuration
The script uses a configuration file that might not exist.
Add existence check:
-changelog=$(git cliff --config examples/detailed.toml --unreleased --strip all) +if [ ! -f "examples/detailed.toml" ]; then + echo "Error: examples/detailed.toml not found" + exit 1 +fi +changelog=$(git cliff --config examples/detailed.toml --unreleased --strip all)Likely invalid or redundant comment.
42-45:⚠️ Potential issueRemove hardcoded Git credentials
Hardcoding Git credentials, especially signing keys, poses a security risk.
Move these to environment variables:
-git -c user.name="zefiro" \ - -c user.email="[email protected]" \ - -c user.signingkey="3ECB750FA11480F8DEF1E1B37614FD08EF9DB246" \ +git -c user.name="${GIT_USER_NAME:-zefiro}" \ + -c user.email="${GIT_USER_EMAIL:[email protected]}" \ + -c user.signingkey="${GIT_SIGNING_KEY:?Git signing key not set}" \Likely invalid or redundant comment.
🧰 Tools
🪛 Gitleaks (8.21.2)
44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
.github/workflows/ci.yml (3)
1-13: LGTM! Well-configured workflow triggers and permissions.The workflow configuration includes comprehensive triggers and appropriate permissions setup.
74-86: LGTM! Well-configured linting setup.The clippy configuration is properly set up with appropriate warning levels.
87-99: LGTM! Proper formatting check configuration.The rustfmt setup is well-configured with appropriate verbosity and scope.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
.github/workflows/ci.yml (3)
3-13: LGTM! Consider restricting permissions furtherThe workflow triggers and permissions are well-configured. However, consider following the principle of least privilege by scoping the permissions more granularly:
permissions: + contents: read pull-requests: write + security-events: write
15-27: Add cargo cache to improve CI performanceThe check job would benefit from caching Rust dependencies to speed up CI runs and make the offline mode more reliable.
Add this step before the cargo checks:
- uses: actions/cache@v3 with: path: | ~/.cargo/bin/ ~/.cargo/registry/index/ ~/.cargo/registry/cache/ ~/.cargo/git/db/ target/ key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} restore-keys: ${{ runner.os }}-cargo-
100-116: Add newline at end of fileAdd a newline at the end of the file to comply with YAML best practices.
done +🧰 Tools
🪛 yamllint (1.35.1)
[error] 116-116: no new line character at the end of file
(new-line-at-end-of-file)
release.sh (4)
13-17: Enhance input validationConsider improving the input validation to:
- Validate the tag format (e.g., ensure it starts with 'v' and follows semver)
- Use explicit exit code
if [ -z "$1" ]; then echo "Please provide a tag." echo "Usage: ./release.sh v[X.Y.Z]" - exit + exit 1 +fi + +if ! echo "$1" | grep -qE "^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)"; then + echo "Error: Tag must follow format v[X.Y.Z] (e.g., v1.0.0)" + exit 1 fi
26-29: Add pre-commit checks and error handlingConsider adding checks for uncommitted changes and command success:
# update the changelog +# Check for uncommitted changes +if ! git diff --quiet HEAD; then + echo "Error: You have uncommitted changes" + exit 1 +fi + git cliff --config cliff.toml --tag "$1" >CHANGELOG.md +if [ $? -ne 0 ]; then + echo "Error: Failed to generate changelog" + exit 1 +fi + git add -A && git commit -m "chore(release): prepare for $1" git show
31-38: Consider externalizing the changelog templateThe template could be moved to a separate file for better maintainability:
Consider creating a
changelog-template.txtfile and loading it:-export GIT_CLIFF_TEMPLATE="\ - {% for group, commits in commits | group_by(attribute=\"group\") %} - {{ group | upper_first }}\ - {% for commit in commits %} - - {% if commit.breaking %}(breaking) {% endif %}{{ commit.message | upper_first }} ({{ commit.id | truncate(length=7, end=\"\") }})\ - {% endfor %} - {% endfor %}" +if [ ! -f "changelog-template.txt" ]; then + echo "Error: changelog-template.txt not found" + exit 1 +fi +export GIT_CLIFF_TEMPLATE="$(cat changelog-template.txt)"
50-52: Enhance user guidance for the final stepsThe final instructions could be more specific about verifying the push:
git tag -v "$1" echo "Done!" -echo "Now push the commit (git push) and the tag (git push --tags)." +echo "Please verify the tag signature above and then:" +echo "1. Push the commit: git push origin main" +echo "2. Push the signed tag: git push origin $1" +echo "3. Verify the push: git ls-remote --tags origin"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)release.sh(1 hunks)
🧰 Additional context used
📓 Learnings (2)
.github/workflows/ci.yml (1)
Learnt from: nsyzrantsev
PR: zefiroproj/zefiro#2
File: .github/workflows/ci.yml:59-59
Timestamp: 2025-01-02T12:41:14.968Z
Learning: The user stated that they do not want the skip directive for the missing `repo::test::git_upstream_remote` test to be fixed in future PRs.
release.sh (1)
Learnt from: nsyzrantsev
PR: zefiroproj/zefiro#2
File: release.sh:42-45
Timestamp: 2025-01-02T12:35:19.240Z
Learning: The user explicitly states that the script in release.sh that has hard-coded Git credentials for user name, email, and signing key is intentionally left as-is. We must not comment on it in future reviews.
🪛 yamllint (1.35.1)
.github/workflows/ci.yml
[error] 116-116: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Gitleaks (8.21.2)
release.sh
48-48: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (5)
.github/workflows/ci.yml (3)
28-36: LGTM! Well-configured typo checkingThe typos job is properly configured with a pinned version of the action.
74-86: LGTM! Strict linting configurationThe clippy configuration is well-structured with warnings treated as errors, promoting high code quality.
87-99: LGTM! Proper formatting checksThe rustfmt configuration is properly set up to ensure consistent code formatting.
release.sh (2)
3-11: Well-structured command verification!The implementation includes proper error handling and helpful installation instructions for missing dependencies.
39-43: Previous review comment still applies
Implemented first version of CI/CD
New Features
git-cliff.Chores