Ref(CI): Unify stub update with android update#5807
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
The changed code Isn't validated by any CI, so that's why I am skipping the |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| cd .. | ||
| yarn build:replay-stubs || true | ||
| ;; | ||
| *) |
There was a problem hiding this comment.
Bug: The yarn build:replay-stubs || true command will silently fail in CI due to missing dependencies, causing a stale replay-stubs.jar to be committed.
Severity: HIGH
Suggested Fix
Remove || true from the command to ensure the script fails loudly if the build fails. Additionally, ensure the CI environment has the necessary dependencies (Node.js, yarn, Java/Gradle) installed to execute the build command successfully.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: scripts/update-android-stubs.sh#L28
Potential issue: The script `update-android-stubs.sh` uses `yarn build:replay-stubs ||
true` to rebuild a JAR file. The CI environment where this script runs likely lacks the
necessary dependencies (Node.js, yarn, Gradle). The `|| true` will suppress the error
when the build command fails, causing the script to appear successful. This results in a
stale `replay-stubs.jar` artifact being committed to the repository, which will be out
of sync with the updated Android SDK version. This discrepancy can lead to build
failures or runtime issues for users of the SDK.
There was a problem hiding this comment.
Correct, but it's better to silently fail the stub code than silently fail the entire CI bump.
We can always check the Bump Action from stub when a PR updates Android but doesn't update the stub.
There was a problem hiding this comment.
LGTM 🚀
Thank you for fixing this @lucas-zimerman 🙇
Before merging let's close #5760 and #5765 and verify that the Android PR is properly recreated when the update-deps runs on main.
@lucas-zimerman The Android bump PR was created successfully #5810 but I think the jar was not created. I think this is due to the Note that I've rerun the job after also deleting the branches |
I think so too, I will remove the yarn dependency and replace it by the gradlew build command. |
📢 Type of change
📜 Description
When updating the Android SDK, it should also update the stub dependency, this PR merge the stub/android update into a single PR.
💡 Motivation and Context
No outdated stub after updating Android SDK.
💚 How did you test it?
Claude
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Close #5230