ci: add PR-build changelog for TestFlight and Play Store#7411
Open
diegolmello wants to merge 2 commits into
Open
ci: add PR-build changelog for TestFlight and Play Store#7411diegolmello wants to merge 2 commits into
diegolmello wants to merge 2 commits into
Conversation
PR builds landed on TestFlight and the Play Store internal track with no release notes, so reviewers could not tell near-identical builds apart. Generate a compact identifying note (PR title + number, then newest-first short-SHA commit rows, merges kept) capped at 500 graphemes with no ellipsis, and attach it on both platforms. PR builds stay internal-only: iOS external distribution is now gated on the develop trigger, not changelog presence. The Build Develop changelog path is unchanged. Claude-Session: https://claude.ai/code/session_01SaW69xLzUYPJdoVJoP2EJQ
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Contributor
WalkthroughAdds a ChangesPR Changelog Pipeline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build-pr.yml (1)
28-30: ⚡ Quick winDisable credential persistence in checkout for this job.
actions/checkoutpersists a token in git config by default. This job only needsGH_TOKENforgh pr view, so turning that off reduces unnecessary credential exposure.Suggested patch
- name: Checkout Repository uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-pr.yml around lines 28 - 30, The actions/checkout action at version v4.3.1 in the Checkout Repository step is currently persisting git credentials by default. To reduce unnecessary credential exposure and align with security best practices, add a with section to the actions/checkout step that sets persist-credentials to false, since this job only requires GH_TOKEN for the gh pr view command and does not need git credentials to be stored in the git config.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/build-pr.yml:
- Around line 28-30: The actions/checkout action at version v4.3.1 in the
Checkout Repository step is currently persisting git credentials by default. To
reduce unnecessary credential exposure and align with security best practices,
add a with section to the actions/checkout step that sets persist-credentials to
false, since this job only requires GH_TOKEN for the gh pr view command and does
not need git credentials to be stored in the git config.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d2d94cb-848e-4e64-8452-88ee584a000e
📒 Files selected for processing (6)
.github/actions/upload-android/action.yml.github/actions/upload-ios/action.yml.github/scripts/generate-pr-changelog.js.github/workflows/build-pr.ymlandroid/fastlane/Fastfileios/fastlane/Fastfile
📜 Review details
🧰 Additional context used
🪛 zizmor (1.25.2)
.github/workflows/build-pr.yml
[warning] 28-29: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 48-48: secrets unconditionally inherited by called workflow (secrets-inherit): this reusable workflow
(secrets-inherit)
🔇 Additional comments (6)
.github/scripts/generate-pr-changelog.js (1)
4-36: LGTM!.github/workflows/build-pr.yml (1)
31-43: LGTM!Also applies to: 45-50, 58-58
.github/actions/upload-android/action.yml (1)
41-53: LGTM!android/fastlane/Fastfile (1)
40-44: LGTM!.github/actions/upload-ios/action.yml (1)
86-89: LGTM!Also applies to: 103-103
ios/fastlane/Fastfile (1)
55-57: LGTM!
diegolmello
temporarily deployed
to
android_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
upload_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.74.0.109140 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNTSAUHUZYHhCIozI1jQp24lvgYDlMCF7-wT0xukI1NiBnl-2pj9IycA5EeEqhIIOe12_zY-bMFIjD-_aGmz |
|
iOS Build Available Rocket.Chat 4.74.0.109141 |
Collapse generate-pr-changelog.js and prepare-changelog.js into changelog.js with two CLI modes (pr / cap). The shared grapheme-cap primitive (capGraphemes) is now defined once; both call sites are updated to invoke the new entry point. Claude-Session: https://claude.ai/code/session_01GXcJgSDQXm89nQHnocDH4r
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
temporarily deployed
to
android_build
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/build-pr.yml (1)
22-26: ⚖️ Poor tradeoffConsider removing the dependency on
run-eslint-and-testfor faster feedback.The changelog generation is independent of lint/test results and could run in parallel. The current dependency adds latency but avoids generating changelogs for PRs that fail linting or tests. If CI speed is a priority, consider removing the
needsconstraint; otherwise, the current design is reasonable for avoiding wasted compute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-pr.yml around lines 22 - 26, The `generate-pr-changelog` job currently depends on the `run-eslint-and-test` job via the `needs` constraint, which adds unnecessary latency since changelog generation is independent of lint and test results. To improve CI speed and enable parallel execution, remove the `needs: [run-eslint-and-test]` line from the `generate-pr-changelog` job definition. This will allow the changelog generation to run concurrently with linting and testing rather than waiting for them to complete first.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/changelog.js:
- Around line 20-39: The formatPrChangelog function assumes commits is an array
with objects containing oid and messageHeadline properties without validating
the input structure. If the input is malformed (corrupted JSON or API schema
changes), the function will crash with cryptic errors. Add input validation at
the beginning of the formatPrChangelog function to verify that commits is an
array and that each commit object contains the required oid and messageHeadline
properties. Throw a descriptive error if validation fails, so callers get clear
feedback about what went wrong instead of undefined errors from trying to access
missing properties.
- Around line 50-53: Add validation after the buildVersion constant assignment
to ensure that process.env.BUILD_VERSION is defined and non-empty before passing
it to the file path in the fs.writeFileSync call. If buildVersion is undefined
or falsy, throw an error with a descriptive message instructing that the
BUILD_VERSION environment variable must be set, or exit the process to prevent
silently creating a file with "undefined" in the path.
- Around line 10-18: The capGraphemes function lacks validation for its
documented precondition that when ellipsis is true, the limit must be at least
4. Add a guard clause at the beginning of the capGraphemes function to validate
that when the ellipsis option is enabled, the limit parameter is at least 4.
This will prevent the function from returning only "..." when the limit is too
small, ensuring the precondition stated in the comment is enforced.
- Around line 46-56: Add try-catch blocks around the file I/O operations and
JSON parsing in both the "pr" and "cap" mode branches to prevent script crashes
from missing files, malformed JSON, or permission errors. Specifically, wrap the
fs.readFileSync call for pr.json and its JSON.parse operation in a try-catch
block that logs a clear error message when parsing or reading fails, and
similarly wrap the fs.readFileSync and fs.writeFileSync operations in the "cap"
mode block with error handling that provides actionable feedback. Each catch
block should log descriptive error messages indicating what failed (file not
found, JSON parse error, permission denied, etc.) rather than letting the
generic Node.js error propagate.
---
Nitpick comments:
In @.github/workflows/build-pr.yml:
- Around line 22-26: The `generate-pr-changelog` job currently depends on the
`run-eslint-and-test` job via the `needs` constraint, which adds unnecessary
latency since changelog generation is independent of lint and test results. To
improve CI speed and enable parallel execution, remove the `needs:
[run-eslint-and-test]` line from the `generate-pr-changelog` job definition.
This will allow the changelog generation to run concurrently with linting and
testing rather than waiting for them to complete first.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f87f335-d963-476c-b1ca-23f6ec1a272b
📒 Files selected for processing (4)
.github/actions/upload-android/action.yml.github/scripts/changelog.js.github/scripts/prepare-changelog.js.github/workflows/build-pr.yml
💤 Files with no reviewable changes (1)
- .github/scripts/prepare-changelog.js
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/upload-android/action.yml
📜 Review details
🔇 Additional comments (5)
.github/scripts/changelog.js (2)
1-8: LGTM!
41-41: LGTM!.github/workflows/build-pr.yml (3)
28-29: LGTM!
31-36: LGTM!
38-43: LGTM!
diegolmello
requested a deployment
to
upload_android
GitHub Actions
Waiting
|
Android Build Available Rocket.Chat 4.74.0.109144 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNQp_0N_YuW-8s88yYMpSxIIkuEDihwStz8mxOUqAR1gnxaMVlESBbp8DKcQemTQcNZ2r8L1Gq4-7pmXaZRg |
|
iOS Build Available Rocket.Chat 4.74.0.109146 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Build PRbuilds landed on TestFlight and the Play Store internal track with no release notes, so reviewers scrolling a list of near-identical PR builds couldn't tell which build belonged to which PR. This adds a compact identifying changelog to PR builds on both platforms.The note format:
Commits are newest-first (HEAD on top), merge commits kept, capped at 500 graphemes with no ellipsis (grapheme-aware so emoji/non-ASCII titles count the way the stores count them). When a PR is large, the oldest rows drop first so the title and latest commit always survive.
What changed:
.github/scripts/changelog.js— one module with two CLI modes.prmode (PR builds) builds the note frompr.jsonviaformatPrChangelog({title, number, commits})and writeschangelog.txt.capmode (develop Android) caps an existingchangelog.txtto the Play Store 500-grapheme limit (497 +...). PR data comes fromgh pr view --json title,number,commits(the PR title isn't in git history, andghavoids shallow-clone concerns). This single module replaces the two former scripts —generate-pr-changelog.jsand the developprepare-changelog.js— which each reimplemented the same grapheme-aware cap.build-pr.yml— thegenerate-pr-changelogjob publishes the note as the samerelease-changelogartifact the develop workflow already uses; both build jobs depend on it.upload-ios/upload-androidactions — download the changelog artifact for the PR trigger too.distribute_external,notify_external_testers, External Testers group) is now gated on the trigger beingdevelop, not on changelog presence. PR builds get the note but stay internal-only — external testers are never emailed an unreviewed PR build.changelog.txtstraight into the Play Store changelog metadata (no ellipsis), and the internalbetalane uploads the changelog while skipping other store metadata.The
Build Developchangelog path is unchanged in behavior — the tag-diffgit logsource, the 500-grapheme ellipsis cap (now viachangelog.js capinstead of a separate prep script), the open-testing lane, and external distribution all stay as before.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1309
How to test or reproduce
This PR's own CI run is the integration test. On the PR build:
Screenshots
Types of changes
Checklist
Further comments
PR builds now carry a changelog, which flips
skip_waiting_for_build_processingtofalseon iOS — the iOS PR upload now waits for App Store build processing (a few minutes to ~30) to attach "What to Test", instead of returning immediately. That's required to set the note; heads-up for anyone watching PR build durations.No automated tests are added for the release tooling, matching the repo convention that GitHub Actions / Fastlane scripts aren't unit-tested here. Verification is the PR CI run itself.
https://claude.ai/code/session_01SaW69xLzUYPJdoVJoP2EJQ
Summary by CodeRabbit