-
Notifications
You must be signed in to change notification settings - Fork 561
Trigger from release branch #3100
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
Trigger from release branch #3100
Conversation
WalkthroughRemoved the required Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
RELEASE.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: browser (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: unit (20.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
RELEASE.md(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: integration (20.x)
- GitHub Check: unit (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
RELEASE.md(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: unit (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
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
🧹 Nitpick comments (1)
RELEASE.md (1)
45-45: Clarify the review-step wording.Please split the run-on sentence so it clearly states when the pipeline pauses and who needs to act during each pause.
-1. The pipeline will pause at the "Print Test/Security scan result and invite Dev team to review" step and also before the final release step, relevant team should review the release details and scan result. +1. The pipeline pauses at the "Print Test/Security scan result and invite Dev team to review" step and again before the final release step. During each pause, the relevant team should review the release details and scan results.
Patel-Raj11
left a comment
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.
LGTM!
| ### **Before triggering a release** | ||
|
|
||
| 1. Create a release branch and update the **`version`** field in `packages/<package_name>/package.json` to the intended release version. | ||
| 1. Create a release branch. A qualified branch name should start with "release-" or "release/", case-insensitive. e.g: `release/[email protected]`, `release-xrpl-4.3.8`, `Release/[email protected]`. |
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.
Hello,
-
Is the updated version name copied into the
mainbranch as well? All the new features are developed and merged into themainbranch of the xrpl.js repository. The version info needs to be updated on bothmainandrelease/<>branches. -
Suppose there is a critical bug in one of the releases. Can I look at the procedure for handling that scenario? Is it documented in any README file? "
Do we need to patch the hotfix to both the release branch and the main branch ? (or) Should we delete the current release branch and cut a rectified-released branch?
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.
#1. A pr is automatically generated for non beta release. for example #3093
#2. I think a new branch should be used, since it is gonna to be a new version. @Patel-Raj11 what do you think?
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.
I think a new branch should be used, since it is gonna to be a new version. @Patel-Raj11 what do you think?
Yes
.github/workflows/release.yml
Outdated
|
|
||
| - name: Validate inputs | ||
| env: | ||
| RELEASE_BRANCH: ${{ github.ref_name }} |
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.
env.RELEASE_BRANCH value is not used in this step. What is the need to define this env variable?
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.
thanks for identifying this, i have removed it.
.github/workflows/release.yml
Outdated
| env: | ||
| PACKAGE_VERSION: "${{ needs.get_version.outputs.package_version }}" | ||
| PACKAGE_NAME: "${{ github.event.inputs.package_name }}" | ||
| RELEASE_BRANCH: "${{ github.ref_name }}" |
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.
Similar to the previous point, I don't see any usages of env.RELEASE_BRANCH in the pre_release job. Why did you define this variable?
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.
thanks for identifying this, i have removed it.
| echo "🔍 Please review the following details before proceeding:" | ||
| echo "📦 Package Name: $PACKAGE_NAME" | ||
| echo "🔖 Package Version: $PACKAGE_VERSION" | ||
| echo "🌿 Release Branch: $RELEASE_BRANCH" |
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.
I was able to trigger a release workflow from a tag-name here: https://github.com/XRPLF/xrpl.js/actions/runs/18320084793
In this case, the github.ref/$RELEASE_BRANCH value points to the tag name, rather than the branch name. The workflow eventually failed in the final step.
For the purposes of debugging, it could be more useful to differentiate between the two cases.
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.
added check at valid input step to make the branch name has to start with release- or release/ regardless of casing. the environment protection rule only allow review and release step to run when the branch name qualifies.
High Level Overview of Change
Trigger deployment from release branch to fix the wrong provenance issue. Fixes #3099
Context of Change
Type of Change
Did you update HISTORY.md?
Test Plan