-
Notifications
You must be signed in to change notification settings - Fork 380
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
test(ci): add deploy status badge to readme and add build ci #1738
Conversation
WalkthroughA new GitHub Actions workflow file named Changes
Poem
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 (
|
If approved, I'll merge so that at the same time i can update the branch protection rules to require the new build CI. |
|
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
🧹 Outside diff range and nitpick comments (3)
.github/workflows/build.yml (3)
1-6
: LGTM! Consider specifying branches for pull requests.The workflow name is clear and descriptive. The trigger events align well with the PR objectives of running the build on all PRs and pushes to the main branch.
Consider specifying the branches for pull requests to ensure the workflow runs only when necessary. You can add this under the
pull_request
event:pull_request: branches: [main]This ensures the workflow runs on PRs targeting the main branch, which is typically the desired behavior.
8-10
: LGTM! Consider pinning the Ubuntu version.The job configuration is appropriate, with a concise name and using the latest Ubuntu.
To ensure consistency across builds, consider pinning the Ubuntu version:
runs-on: ubuntu-22.04This prevents potential issues if a new Ubuntu version is released with breaking changes.
21-22
: LGTM! Consider adding post-build steps.The build step correctly uses "yarn build" for a VitePress site.
Consider adding the following improvements:
- Verify the build output, e.g., check if the output directory exists.
- Upload the build artifacts for potential use in deployment or verification.
Here's an example of how you could implement these suggestions:
- name: Build with VitePress run: yarn build - name: Verify build output run: | if [ ! -d ".vitepress/dist" ]; then echo "Build output directory not found" exit 1 fi - name: Upload build artifacts uses: actions/upload-artifact@v3 with: name: dist path: .vitepress/distThese additions will help ensure the build process is successful and make the artifacts available for further use if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/build.yml (1 hunks)
- README.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🔇 Additional comments (2)
.github/workflows/build.yml (2)
12-20
: LGTM! Environment setup looks solid.The environment setup steps are well-configured:
- Using the latest major versions of actions/checkout and actions/setup-node.
- Node.js version is pinned to 18 for consistency.
- Yarn caching is enabled for better performance.
- Dependency installation uses Yarn, consistent with the caching strategy.
1-22
: Overall implementation looks good, but deploy status badge is missing.The workflow successfully implements a build stage that runs on all PRs, aligning with the main PR objective. It provides a solid foundation for CI with room for minor improvements (as suggested in previous comments).
However, the PR objective of adding a deploy status badge to the README is not addressed in this workflow. To verify if this has been implemented elsewhere, let's check the README file:
If the above command doesn't return any results, consider adding the deploy status badge to the README as mentioned in the PR objectives.
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.
Thank you! Resolves #1739
Overview
Currently there is no branch protect from a CI perspective. This PR as the build stage without deployment so that it can run on all PRs and be require to merge.
Additionally this adds the deploy status badge to the README for visibility if it were to fail.
Summary by CodeRabbit
New Features
Documentation