-
Notifications
You must be signed in to change notification settings - Fork 82
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
fix: next release job #1529
fix: next release job #1529
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
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 (
|
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 (2)
.circleci/config.yml (2)
274-274
: Consider usingxlarge
resource class for consistencyWhile adding
resource_class: large
addresses the resource constraints, consider usingxlarge
to align with other resource-intensive jobs likebuild
andtest-integration-with-smart-contracts
. This would ensure consistent performance across the pipeline, especially during the publishing process which involves running lerna and npm operations.- resource_class: large + resource_class: xlarge
Line range hint
279-281
: Use CircleCI's built-in GitHub host verificationInstead of manually adding GitHub's SSH key to known hosts, consider using CircleCI's built-in features:
- For host verification only: Remove this step as CircleCI already trusts GitHub hosts
- For SSH operations: Use the
add_ssh_keys
stepIf SSH operations are needed, replace with:
- add_ssh_keys: fingerprints: - "xx:xx:xx:xx:xx:xx:xx:xx" # Add your SSH key fingerprint
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.circleci/config.yml
(1 hunks)
🔇 Additional comments (2)
.circleci/config.yml (2)
Line range hint 348-361
: LGTM: Workflow configuration ensures quality releases
The workflow configuration is well-structured:
- All tests and checks must pass before release
- Branch filtering ensures controlled releases
- Proper context usage for secrets
Line range hint 282-299
: Verify impact of removed lerna build step
The removal of the lerna build step aligns with the PR objective to avoid redundant builds. However, please verify:
- The built artifacts from the
build
job are correctly persisted and attached - The publishing process doesn't require fresh builds
✅ Verification successful
Let me gather more specific information about the workspace persistence and attachment configurations.
Let me check the build and next-release jobs configuration more specifically.
Based on the gathered information, I can now provide the final response:
Removal of lerna build step is safe ✅
The workspace configuration shows that:
- The
build
job correctly persists all artifacts usingpersist_to_workspace
- The
next-release
job attaches the workspace and hasbuild
as a requirement in the workflow - All built artifacts from the previous build step are available in the workspace when publishing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if lerna publish requires fresh builds
rg -A 5 'lerna publish' packages/*/package.json
# Check if workspace is correctly persisted
rg 'persist_to_workspace|attach_workspace' .circleci/config.yml
Length of output: 1200
Script:
#!/bin/bash
# Check workspace persistence and attachment details
rg -B 2 -A 3 'persist_to_workspace:|attach_workspace:' .circleci/config.yml
# Check build job configuration
rg -B 5 -A 10 'jobs:.*build:' .circleci/config.yml
Length of output: 1664
Script:
#!/bin/bash
# Get the build job and next-release job configurations
rg -B 2 -A 15 'build:|next-release:' .circleci/config.yml
# Check workflow configuration to understand job dependencies
rg -B 2 -A 15 'workflows:' .circleci/config.yml
Length of output: 2467
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.
Looks good to me assuming the resulting packages all have dist folders as expected.
Problem
Calling clean build again in the next-release job is necessary. Also the container needs more resource.
Description of the changes
update CircleCI config to use large resource class and remove lerna build step
Summary by CodeRabbit
New Features
Improvements
next-release
job to optimize performance.