Release pipeline #876
Release pipeline #876
Conversation
fix poetry installation
shichengripple001
requested review from
achowdhry-ripple,
ahoffman-ripple,
ckeshava,
mvadari and
pdp2121
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/faucet_test.yml(1 hunks).github/workflows/integration_test.yml(1 hunks).github/workflows/publish_to_pypi.yml(0 hunks).github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)pyproject.toml(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/publish_to_pypi.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/release.yml
135-135: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 LanguageTool
RELEASE.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...d ship a new xrpl-py version using the `Publish xrpl-py 🐍 distribution 📦 to P...
(QB_NEW_EN)
[grammar] ~4-~4: There might be a mistake here.
Context: ...📦 to PyPIGitHub Actions workflow (see.github/workflows/release.yml`). It mir...
(QB_NEW_EN)
[uncategorized] ~4-~4: The official name of this software platform is spelled with a capital “H”.
Context: ...? to PyPIGitHub Actions workflow (see.github/workflows/release.yml`). It mirrors the...
(GITHUB)
[grammar] ~5-~5: There might be a mistake here.
Context: ...rors the automation and reviews that run when the workflow is manually dispatched...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...ications go to #xrpl-py). - Reviewers from dev team and infosec team to approve Gi...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ... #xrpl-py). - Reviewers from dev team and infosec team to approve GitHub environm...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...ironment gates and review pull requests. - PyPI Trusted Publisher to trust the work...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...tiates between beta/pre-release versions and standard releases by reading version...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...lease versions and standard releases by reading version under [project] section from py...
(QB_NEW_EN)
[uncategorized] ~22-~22: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... - The GitHub Release is created with --prerelease. - The latest tag on GitHub rema...
(EN_WORD_COHERENCY)
[grammar] ~22-~22: There might be a mistake here.
Context: ... Release is created with --prerelease. - The latest tag on GitHub remains uncha...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...efault download). - Stable release: - A PR from the release branch to main i...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...eview and merge after PyPI verification. - The GitHub Release is created with `--la...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...elease Branch 1. Create release branch using name pattern release-x.y.z (or `relea...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...rn release-x.y.z (or release/x.y.z). 2. Bump project.version inside `pyproject...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...he version already exists, if the branch name does not match the required prefix....
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...ine is: | Stage | Purpose (key steps) | | --- | --- | | input-validate | Check...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...ge | Purpose (key steps) | | --- | --- | | input-validate | Checks branch namin...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...release is a beta (a, b, or rc). | | faucet-tests, integration-tests | ...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...fficial-releaseenvironment approval. | |publish-to-pypi` | Downloads the buil...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...o-pypi` | Downloads the built artifacts from previous step, enforces single-run (no ...
(QB_NEW_EN)
[uncategorized] ~59-~59: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... creates or updates the GitHub Release (--prerelease for beta versions, --latest for stab...
(EN_WORD_COHERENCY)
[grammar] ~65-~65: There might be a mistake here.
Context: ...e the first-review environment gate. - Security review: After the Dev gate is...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...om scratch if it fails after publishing. 2. Confirm the new version is visible on Py...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ... the pre-release flag if applicable). 4. Merge the automated release PR into `mai...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...only). Do this after you verify PyPI. 5. Create any follow-up housekeeping PRs (e...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...gs origin`), and Artifactory references. - GitHub release creation fails: Look at...
(QB_NEW_EN)
[uncategorized] ~92-~92: The official name of this software platform is spelled with a capital “H”.
Context: ...tomation needs adjustments, update both .github/workflows/release.yml and this guide s...
(GITHUB)
⏰ 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). (7)
- GitHub Check: Integration test (3.9)
- GitHub Check: Integration test (3.13)
- GitHub Check: Integration test (3.10)
- GitHub Check: Integration test (3.11)
- GitHub Check: Integration test (3.8)
- GitHub Check: Integration test (3.12)
- GitHub Check: semgrep-cloud-platform/scan
resolve conflict/fix slack message
resolve conflict/fix slack message
shichengripple001
force-pushed
the
release-pipeline
branch
from
4e752ab to
9b6d9fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
134-139: Bump cache action to v4 (Node20).actions/cache@v3 runs on deprecated Node16 and will fail. Use v4.
- - name: Load cached .local - id: cache-poetry - uses: actions/cache@v3 + - name: Load cached .local + id: cache-poetry + uses: actions/cache@v4 with: path: /home/runner/.local key: dotlocal-${{ env.POETRY_VERSION }}-${{ hashFiles('poetry.lock') }}
590-607: Fix Slack success message: set PACKAGE_NAME, use encoded tag, and correct typo.PACKAGE_NAME is empty; enc_tag is computed but unused; “successfully” misspelled.
env: SLACK_TOKEN: ${{ secrets.SLACK_TOKEN }} REPO: ${{ github.repository }} - PACKAGE_NAME: ${{ env.PACKAGE_NAME }} + PACKAGE_NAME: xrpl-py PACKAGE_VERSION: ${{ env.PACKAGE_VERSION }} TAG: ${{ env.PACKAGE_VERSION }} run: | set -euo pipefail # Build release URL from tag (URL-encoded to handle '@' etc.) TAG="${TAG:-${PACKAGE_VERSION}}" enc_tag="$(printf '%s' "$TAG" | jq -sRr @uri)" - RELEASE_URL="https://github.com/$REPO/releases/tag/$TAG" + RELEASE_URL="https://github.com/$REPO/releases/tag/$enc_tag" - text="${PACKAGE_NAME} ${PACKAGE_VERSION} has been succesfully released and published to pypi. Release URL: ${RELEASE_URL}" + text="${PACKAGE_NAME} ${PACKAGE_VERSION} has been successfully released and published to PyPI. Release URL: ${RELEASE_URL}" text="${text//\\n/ }"Optional: define PACKAGE_NAME once at workflow level to cover all steps:
# Near the top of the workflow env: PACKAGE_NAME: xrpl-py
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
80-89: Use robust prerelease detection (PEP 440).Regex (a|b|rc) can produce false positives. Consider packaging.version to detect prereleases accurately.
- name: Detect release kind id: detect_release_kind run: | set -euo pipefail - VERSION="${{ steps.package_version.outputs.version }}" - if [[ "$VERSION" =~ (a|b|rc) ]]; then - echo "is_beta_release=true" >> "$GITHUB_OUTPUT" - else - echo "is_beta_release=false" >> "$GITHUB_OUTPUT" - fi + python - << 'PY' +import os +from packaging.version import Version +v = os.environ["VERSION"] +is_pre = Version(v).is_prerelease +print(f"is_beta_release={'true' if is_pre else 'false'}") +PY + | tee -a "$GITHUB_OUTPUT" env: + PIP_DISABLE_PIP_VERSION_CHECK: "1" + PIP_NO_PYTHON_VERSION_WARNING: "1" + # Ensure packaging is available (runner usually has it, but install if needed) + shell: bash + continue-on-error: falseOr keep bash and tighten regex: ^\d+(.\d+)*((a|b|rc)\d+)$
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/release.yml
135-135: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (2)
- GitHub Check: Integration test (3.13)
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
RELEASE.md (1)
3-5: Fix opening sentence grammar.Please update to “This guide document describes …” so the verb agrees with the subject and keep the capital “H” in GitHub, per the earlier feedback.
-This guide document describe how to cut and ship a new `xrpl-py` version using the +This guide document describes how to cut and ship a new `xrpl-py` version using the `Publish xrpl-py 🐍 distribution 📦 to PyPI` GitHub Actions workflow (see `.github/workflows/release.yml`).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
RELEASE.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
RELEASE.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...d ship a new xrpl-py version using the `Publish xrpl-py 🐍 distribution 📦 to P...
(QB_NEW_EN)
[uncategorized] ~4-~4: The official name of this software platform is spelled with a capital “H”.
Context: ...? to PyPIGitHub Actions workflow (see.github/workflows/release.yml`). ## 0. Config...
(GITHUB)
[grammar] ~5-~5: There might be a mistake here.
Context: ...w (see .github/workflows/release.yml). ## 0. Configurations required for this pipe...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...ications go to #xrpl-py). - Reviewers from dev team and infosec team to approve Gi...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... #xrpl-py). - Reviewers from dev team and infosec team to approve GitHub environm...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...ironment gates and review pull requests. - PyPI Trusted Publisher to trust the work...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...tiates between beta/pre-release versions and standard releases by reading version...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...lease versions and standard releases by reading version under [project] section from py...
(QB_NEW_EN)
[uncategorized] ~21-~21: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... - The GitHub Release is created with --prerelease. - The latest tag on GitHub rema...
(EN_WORD_COHERENCY)
[grammar] ~21-~21: There might be a mistake here.
Context: ... Release is created with --prerelease. - The latest tag on GitHub remains uncha...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...efault download). - Stable release: - A PR from the release branch to main i...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...eview and merge after PyPI verification. - The GitHub Release is created with `--la...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...elease Branch 1. Create release branch using name pattern release-x.y.z (or `relea...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...rn release-x.y.z (or release/x.y.z). 2. Bump project.version inside `pyproject...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...he version already exists, if the branch name does not match the required prefix....
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...ine is: | Stage | Purpose (key steps) | | --- | --- | | input-validate | Check...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...ge | Purpose (key steps) | | --- | --- | | input-validate | Checks branch namin...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...release is a beta (a, b, or rc). | | faucet-tests, integration-tests | ...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...fficial-releaseenvironment approval. | |publish-to-pypi` | Downloads the buil...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...o-pypi` | Downloads the built artifacts from previous step, enforces single-run (no ...
(QB_NEW_EN)
[uncategorized] ~58-~58: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... creates or updates the GitHub Release (--prerelease for beta versions, --latest for stab...
(EN_WORD_COHERENCY)
[grammar] ~64-~64: There might be a mistake here.
Context: ...e the first-review environment gate. - Security review: After the Dev gate is...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...om scratch if it fails after publishing. 2. Confirm the new version is visible on Py...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... the pre-release flag if applicable). 4. Merge the automated release PR into `mai...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...only). Do this after you verify PyPI. 5. Create any follow-up housekeeping PRs (e...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...gs origin`), and Artifactory references. - GitHub release creation fails: Look at...
(QB_NEW_EN)
[uncategorized] ~91-~91: The official name of this software platform is spelled with a capital “H”.
Context: ...tomation needs adjustments, update both .github/workflows/release.yml and this guide s...
(GITHUB)
⏰ 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 test (3.10)
- GitHub Check: Integration test (3.13)
- GitHub Check: Integration test (3.8)
- GitHub Check: Integration test (3.11)
- GitHub Check: Integration test (3.9)
- GitHub Check: Integration test (3.12)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
| The workflow automatically differentiates between beta/pre-release versions | ||
| and standard releases by reading version under [project] section from pyproject.toml: | ||
|
|
There was a problem hiding this comment.
Tighten wording around version detection.
Add the missing articles so the sentence reads smoothly: “by reading the version under the [project] section from pyproject.toml.”
-The workflow automatically differentiates between beta/pre-release versions
-and standard releases by reading version under [project] section from pyproject.toml:
+The workflow automatically differentiates between beta/pre-release versions
+and standard releases by reading the version under the [project] section from `pyproject.toml`:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The workflow automatically differentiates between beta/pre-release versions | |
| and standard releases by reading version under [project] section from pyproject.toml: | |
| The workflow automatically differentiates between beta/pre-release versions | |
| and standard releases by reading the version under the [project] section from `pyproject.toml`: |
🧰 Tools
🪛 LanguageTool
[grammar] ~16-~16: There might be a mistake here.
Context: ...tiates between beta/pre-release versions and standard releases by reading version...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...lease versions and standard releases by reading version under [project] section from py...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In RELEASE.md around lines 16 to 18, the sentence lacks the definite article and
reads awkwardly; update it to include the missing articles so it reads: "by
reading the version under the [project] section from `pyproject.toml`." Edit
that line to insert "the" before "version" and ensure the phrase mentions "the
[project] section" and "`pyproject.toml`" exactly as shown.
| - **Security review**: After the Dev gate is cleared, the workflow pauses at | ||
| `official-release`. Security reviewers receive a Slack ping and must review the vulnerabilities approve | ||
| that environment gate. |
There was a problem hiding this comment.
Fix sentence to include the required verb.
Clarify that security reviewers both review the reports and approve the gate.
-`official-release`. Security reviewers receive a Slack ping and must review the vulnerabilities approve
- that environment gate.
+`official-release`. Security reviewers receive a Slack ping and must review the vulnerability reports and
+ approve that environment gate.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Security review**: After the Dev gate is cleared, the workflow pauses at | |
| `official-release`. Security reviewers receive a Slack ping and must review the vulnerabilities approve | |
| that environment gate. | |
| - **Security review**: After the Dev gate is cleared, the workflow pauses at | |
| `official-release`. Security reviewers receive a Slack ping and must review the vulnerability reports and | |
| approve that environment gate. |
🤖 Prompt for AI Agents
In RELEASE.md around lines 65 to 67, the sentence "Security reviewers receive a
Slack ping and must review the vulnerabilities approve that environment gate."
is missing the required verb and is unclear; update it to a clear phrasing such
as "Security reviewers receive a Slack ping and must review the vulnerability
reports and approve the environment gate." so it explicitly states that
reviewers both review the reports and approve the gate.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
RELEASE.md (2)
65-67: Fix incomplete sentence on security reviews.The sentence is missing a verb between "vulnerabilities" and "approve". Update it for grammatical correctness:
-`official-release`. Security reviewers receive a Slack ping and must review the vulnerabilities approve - that environment gate. +`official-release`. Security reviewers receive a Slack ping and must review the vulnerability + reports and approve that environment gate.
16-17: Add missing articles for clarity.The sentence reads awkwardly without articles. Apply this fix to improve readability:
-The workflow automatically differentiates between beta/pre-release versions -and standard releases by reading version under [project] section from pyproject.toml: +The workflow automatically differentiates between beta/pre-release versions +and standard releases by reading the version under the [project] section from `pyproject.toml`:.github/workflows/release.yml (2)
557-576: Add --target to gh release create to align tag with built artifacts.The
gh release createcommand defaults to tagging the current HEAD on the default branch, which could misalign the tag from the artifacts if commits were pushed between the build step and this release step. Specify the exact commit:run: | set -euo pipefail ARGS=( "$PACKAGE_VERSION" --repo "$GITHUB_REPOSITORY" --generate-notes + --target "$GITHUB_SHA" ) if [ "${IS_BETA_RELEASE:-false}" = "true" ]; then ARGS+=(--prerelease) else ARGS+=(--latest) fi gh release create "${ARGS[@]}" || { echo "::error::Failed to create release" exit 1 }
496-499: Add attestations permission for PyPI publish.Line 519 uses
attestations: truebut the permissions block lacksattestations: write, which will cause the action to fail:permissions: # More information about Trusted Publishing and OpenID Connect: https://blog.pypi.org/posts/2023-04-20-introducing-trusted-publishers/ - id-token: write # IMPORTANT: mandatory for trusted publishing + id-token: write # mandatory for trusted publishing + attestations: write # required when attestations: true
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
RELEASE.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...d ship a new xrpl-py version using the `Publish xrpl-py 🐍 distribution 📦 to P...
(QB_NEW_EN)
[uncategorized] ~4-~4: The official name of this software platform is spelled with a capital “H”.
Context: ...? to PyPIGitHub Actions workflow (see.github/workflows/release.yml`). ## 0. Config...
(GITHUB)
[grammar] ~5-~5: There might be a mistake here.
Context: ...w (see .github/workflows/release.yml). ## 0. Configurations required for this pipe...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...and#ripplex-security`). - Reviewers from dev team and infosec team to approve Gi...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...x-security`). - Reviewers from dev team and infosec team to approve GitHub environm...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...ironment gates and review pull requests. - PyPI Trusted Publisher to trust the work...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...tiates between beta/pre-release versions and standard releases by reading version...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...lease versions and standard releases by reading version under [project] section from py...
(QB_NEW_EN)
[uncategorized] ~21-~21: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... - The GitHub Release is created with --prerelease. - The latest tag on GitHub rema...
(EN_WORD_COHERENCY)
[grammar] ~21-~21: There might be a mistake here.
Context: ... Release is created with --prerelease. - The latest tag on GitHub remains uncha...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...efault download). - Stable release: - A PR from the release branch to main i...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...eview and merge after PyPI verification. - The GitHub Release is created with `--la...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...elease Branch 1. Create release branch using name with prefix release- (or `releas...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ch 1. Create release branch using name with prefix release- (or release/). 2. B...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ... with prefix release- (or release/). 2. Bump project.version inside `pyproject...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...he version already exists, if the branch name does not match the required prefix....
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...ine is: | Stage | Purpose (key steps) | | --- | --- | | input-validate | Check...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...ge | Purpose (key steps) | | --- | --- | | input-validate | Checks branch namin...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...release is a beta (a, b, or rc). | | faucet-tests, integration-tests | ...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...fficial-releaseenvironment approval. | |publish-to-pypi` | Downloads the buil...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...o-pypi` | Downloads the built artifacts from previous step, enforces single-run (no ...
(QB_NEW_EN)
[uncategorized] ~58-~58: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... creates or updates the GitHub Release (--prerelease for beta versions, --latest for stab...
(EN_WORD_COHERENCY)
[grammar] ~64-~64: There might be a mistake here.
Context: ...e the first-review environment gate. - Security review: After the Dev gate is...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...om scratch if it fails after publishing. 2. Confirm the new version is visible on Py...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... the pre-release flag if applicable). 4. Merge the automated release PR into `mai...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...only). Do this after you verify PyPI. 5. Create any follow-up housekeeping PRs (e...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...gs origin`), and Artifactory references. - GitHub release creation fails: Look at...
(QB_NEW_EN)
[uncategorized] ~91-~91: The official name of this software platform is spelled with a capital “H”.
Context: ...tomation needs adjustments, update both .github/workflows/release.yml and this guide s...
(GITHUB)
⏰ 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 test (3.8)
- GitHub Check: Integration test (3.11)
- GitHub Check: Integration test (3.10)
- GitHub Check: Integration test (3.9)
- GitHub Check: Integration test (3.13)
- GitHub Check: Integration test (3.12)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/faucet_test.yml (1)
1-52: Consider exposing workflow inputs for caller customization.Adding
workflow_callallows reuse from the release pipeline, but the workflow currently defines no inputs. This means external workflows cannot customize the Python versions or other parameters. If the release pipeline needs flexibility (e.g., testing against a subset of Python versions), consider exposinginputsin theworkflow_calltrigger.Example enhancement:
on: push: branches: [main] workflow_dispatch: workflow_call: + inputs: + python-versions: + description: "Python versions to test (JSON array)" + type: string + default: '["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]'Then update line 19 to use the input:
- python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"] + python-version: ${{ fromJson(inputs.python-versions) }}This is optional; if the faucet tests always run with the full matrix, no changes are needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/faucet_test.yml(1 hunks).github/workflows/integration_test.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/integration_test.yml
⏰ 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). (9)
- GitHub Check: Integration test (3.11)
- GitHub Check: Integration test (3.10)
- GitHub Check: Integration test (3.8)
- GitHub Check: Integration test (3.12)
- GitHub Check: Integration test (3.9)
- GitHub Check: Integration test (3.14)
- GitHub Check: Integration test (3.13)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
.github/workflows/faucet_test.yml (1)
7-7: ✓ Correct addition ofworkflow_calltrigger.The
workflow_calltrigger syntax is valid and allows this workflow to be invoked by other workflows (e.g., the release pipeline). This change aligns with the PR's goal of creating a reusable, multi-stage release workflow.
| @@ -0,0 +1,92 @@ | |||
| # xrpl-py Release Playbook | |||
There was a problem hiding this comment.
- Since we have this automated release steps in place here, let us remove those from CONTRIBUTING.md
- We should move manual release steps from CONTRIBUTING.md to this file (in case pipeline fails and we need to release the package manually). I think CONTRIBUTING.md should not have any release steps.
There was a problem hiding this comment.
in case pipeline fails and we need to release the package manually
The manual release workflow publish_to_pypi.yml has been removed. Should we keep it as a backup? Also, we need to update it to ensure that the steps are still working.
There was a problem hiding this comment.
The manual release workflow publish_to_pypi.yml has been removed. Should we keep it as a backup?
This was also a light weight automation. The publish steps is still happening from the CI. I think if one wants to release it manually they can follow the steps that are currently mentioned in CONTRIBUTING.md
There was a problem hiding this comment.
if this workflow fail, publish_to_pypi.yml should also fail. they both use trusted publisher, I think manual release should run from local, we should just assume github action went down.
There was a problem hiding this comment.
Thanks both for the clarification. In that case, as Raj suggested, should we update the release-process
section to remove the automated release instructions and keep only the manual release steps? We could also move those steps to a RELEASE.md file or simply reference them as a backup process for situations where automation can’t be used.
| set -euo pipefail | ||
| python3 -m venv /tmp/tomlcli | ||
| /tmp/tomlcli/bin/pip install --upgrade pip | ||
| /tmp/tomlcli/bin/pip install toml-cli |
There was a problem hiding this comment.
We should pin toml-cli to a specific version.
| echo "version=${VERSION}" >> "${GITHUB_OUTPUT}" | ||
| echo "Detected package version: ${VERSION}" | ||
| - name: Detect release kind |
There was a problem hiding this comment.
nit: Is it clearer to name it Determine release type or Detect beta release?
| uses: actions/setup-python@v5 | ||
| with: | ||
| # Use the lowest supported version of Python for CI/CD | ||
| python-version: "3.9" |
There was a problem hiding this comment.
Can the version be determined programmatically? It will change over time.
There was a problem hiding this comment.
Currently the lowest version supported is 3.8.
| @@ -0,0 +1,92 @@ | |||
| # xrpl-py Release Playbook | |||
There was a problem hiding this comment.
in case pipeline fails and we need to release the package manually
The manual release workflow publish_to_pypi.yml has been removed. Should we keep it as a backup? Also, we need to update it to ensure that the steps are still working.
| 5. Create any follow-up housekeeping PRs (e.g., bumping `dev` version or | ||
| updating docs) if needed. |
There was a problem hiding this comment.
Since this file is the source of truth for the release process, can we be specific here about what need to do to complete a release? From https://github.com/XRPLF/xrpl-py/blob/main/CONTRIBUTING.md#release-process, I can see that we will need to do the following additional steps:
- Send an email to xrpl-announce.
- Post an announcement in the XRPL Discord #python channel with a link to the changes and highlighting key changes.
@Patel-Raj11 Do we need to update reference doc as well?
There was a problem hiding this comment.
+1 to above two points.
Do we need to update reference doc as well?
Those gets automatically picked up based on Github tags.
| "Bug Tracker" = "https://github.com/XRPLF/xrpl-py/issues" | ||
|
|
||
| [tool.poetry] | ||
| name = "xrpl-py" |
There was a problem hiding this comment.
If there a specific reason we need to have name and description here as well? If it's not required for release workflow, lets revert changes in this file.
| echo "is_beta_release=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| faucet-tests: |
There was a problem hiding this comment.
unit_test.yml should be invoked as well.
| needs: | ||
| - input-validate | ||
| - faucet-tests | ||
| - integration-tests |
There was a problem hiding this comment.
unit_test.yml should be a dependency here as well.
| with: | ||
| path: /home/runner/.local | ||
| key: dotlocal-${{ env.POETRY_VERSION }}-${{ hashFiles('poetry.lock') }} | ||
| - name: Install poetry |
There was a problem hiding this comment.
I think it's better to be consistent with the way poetry is installed in all the workflows.
Also, the current steps don't use the cache-poetry. We should follow integration_test workflow and mimic how to reuse the cache.
| - name: Install CycloneDX Python tool | ||
| run: | | ||
| set -euo pipefail | ||
| python -m pip install --upgrade cyclonedx-bom |
There was a problem hiding this comment.
We should pin this dependency as well.
| name: Summarize release and request Dev review | ||
| runs-on: ubuntu-latest | ||
| needs: | ||
| - input-validate |
There was a problem hiding this comment.
Add unit tests dependency here as well.
| inputs: >- | ||
| ./dist/*.tar.gz | ||
| ./dist/*.whl | ||
| - name: Create GitHub Release |
There was a problem hiding this comment.
Can we just use this workflow that is readily available to create Github release? I feel the current one is very verbose.
https://github.com/XRPLF/xrpl.js/blob/3e867b09533dfa087643436eaa0c44f688d54dd1/.github/workflows/release.yml#L590
There was a problem hiding this comment.
@shichengripple001 Once these comments are resolved , can you please share links to the following here:
- Sample Github release tag on your fork.
- Sample xrpl-py package that got published.
3 participants
High Level Overview of Change
Revamp release pipeline to run tests, scan for vulnerabilities, request for review
Context of Change
Type of Change
Did you update CHANGELOG.md?
Test Plan