-
Notifications
You must be signed in to change notification settings - Fork 8
[AINFRA-1462] Add new platforms to upload_build_to_apps_cdn
#669
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
Conversation
upload_build_to_apps_cdnupload_build_to_apps_cdn
| 'Windows', | ||
| 'Microsoft Store', | ||
| 'Windows - x86', | ||
| 'Windows - ARM64', | ||
| 'Microsoft Store - x86', | ||
| 'Microsoft Store - ARM64', |
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.
ℹ️ As discussed in https://linear.app/a8c/issue/AINFRA-1457/update-appscdn-backend-and-release-toolkit-for-new-windows#comment-646bbf97, we've agreed to get rid of the original plain 'Windows' and 'Microsoft Store' values (as opposed to keeping them for backwards compatibility) since Studio was the only client product to use those old non-architecture-specific values in the first place.
| ### New Features | ||
|
|
||
| _None_ | ||
| - `upload_build_to_apps_cdn`: Update the list of valid values for `platform` to now support _both_ `x86` and `ARM64` for the `Microsoft Store` and `Windows` platforms. [#669] |
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 pondered marking this as breaking change, given the 'Windows' and 'Microsoft Store' values that were previously valid are now invalid.
That being said, because Studio is the only client that used those values, and will adopt the new release-toolkit as soon as this PR is merged and a new version is released—since they are the ones who asked for those values—I figured it was not worth marking this as a breaking change per se.
sejas
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.
Thanks for making the change to the release toolkit. The code looks good to me.
| begin | ||
| $skip_magick = false | ||
| require 'RMagick' | ||
| rescue LoadError |
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.
Unrelated to this PR, but was necessary to fix a bug preventing me to run the tests (rspec) locally when the version of RMagic installed on my Mac was not the one expected.
Closes AINFRA-1462
What does it do?
Add new
Windows - ARM64andMicrosoft Store - ARM64platforms toupload_build_to_apps_cdnChecklist before requesting a review
bundle exec rubocopto test for code style violations and recommendations.specs/*_spec.rb) if applicable.bundle exec rspecto run the whole test suite and ensure all your tests pass.CHANGELOG.mdfile to describe your changes under the appropriate existing###subsection of the existing## Trunksection.If applicable, add an entry in theMIGRATION.mdfile to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.