-
Notifications
You must be signed in to change notification settings - Fork 25
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
ci: release please support #241
Conversation
This pull request has been linked to Shortcut Story #234641: Android release please. |
options.addStringOption("Xwerror", "-quiet") | ||
} | ||
} | ||
|
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.
After these action changes, this "treat warnings as errors" step started failing the build. I'm unsure why this was not failing before when using Circle CI.
I fixed as many of the warnings as I could, but could not resolve warnings on autogenerated files such as BuildConfig.java. After about 3 hours of investigating, decided to eliminate this to move on to other priorities. Ultimately this PR does fix ~80 out of the ~85 warnings that it was giving.
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.
Ok. We should probably file a task to come back to it. Ideally we could ignore those generated files, but I know that tooling isn't always designed with people actually using it in mind.
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.
How about lumping it in with the mono repo work?
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.
Created 235492 to track this
run_tests: true | ||
# TODO: revert dry run put in place during testing | ||
dry_run: true | ||
prerelease: false |
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.
The TODO will be eliminated later after it has been tested on main.
# android_api_level: ['21','25','30','34'] | ||
# java_version: ['11', '17'] | ||
android_api_level: ['25'] | ||
java_version: ['17'] |
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.
Would like to address this TODO in a subsequent PR.
Still need to add contract test invocation in the CI action. Done |
.github/actions/ci/action.yml
Outdated
run: make build-contract-tests | ||
|
||
- name: Perform Instrumented Tests | ||
uses: reactivecircus/android-emulator-runner@v2 |
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 would use a hash on this one instead of the v2.
Requirements
I have added test coverage for new or changed functionality
No behavioral changes in this PR.
I have followed the repository's pull request submission guidelines
I have validated my changes against all supported platform versions
Ongoing, have TODO related to API/Java version combinations, but would like to get this portion merged to unblock utilizing release please for a pending bugfix. Planning to fix the matrices in a subsequent PR.
Related issues
SC-234641
Describe the solution you've provided
Adds CI, docs, publishing, and release please actions.