Skip to content
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

Another try to add AppX to artifact links #733

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

wojtekn
Copy link
Contributor

@wojtekn wojtekn commented Dec 11, 2024

Related issues

Proposed Changes

  • I propose to add AppX binaries to artifact links to simplify Microsoft Store submission steps.

Testing Instructions

  • N/A

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@wojtekn wojtekn requested a review from a team December 11, 2024 09:57
@wojtekn wojtekn self-assigned this Dec 11, 2024
@wojtekn wojtekn force-pushed the fix/normalized-version-suffix branch from 5919dbe to a445748 Compare December 11, 2024 10:25
@@ -93,6 +93,7 @@ def distribute_builds(
# If we are distributing a build without a tag, i.e. a development build, we also want to update the latest build reference for distribution.
update_latest = release_tag.nil?
suffix = release_tag.nil? ? commit_hash : release_tag
normalized_version = suffix.gsub(/^v|-.*/, '')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added v as without that build script tried to access Studio v1.3.0.0.appx and not Studio 1.3.0.0.appx.

@@ -248,6 +249,7 @@ steps:
buildkite-agent artifact download "*.zip" .
buildkite-agent artifact download "*.dmg" .
buildkite-agent artifact download "*.exe" .
buildkite-agent artifact download "*.appx" .
Copy link
Contributor Author

@wojtekn wojtekn Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build step neees to download the appx files before distributing them.

name: 'Windows (Appx)'
},
windows_appx_unsigned: {
binary_path: File.join(BUILDS_FOLDER, 'Studio-appx-unsigned', "Studio #{normalized_version}.0 unsigned.appx"),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach doesn't work for dev builds as the build script looks for the Studio 287a659.0.appx file name, but the actual filename received from the previous step is Studio 1.3.0.0.appx.

Can we remove the version from the files to match behavior for other artifacts?

@wojtekn wojtekn changed the title Fix normalized version suffix Another try to add AppX to artifact links Dec 11, 2024
@wojtekn wojtekn requested a review from mokagio December 11, 2024 10:34
Copy link
Member

@sejas sejas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I left a suggestion about removing the signed version.

Not sure if this will add the link to the Slack bot, but it would be a nice to have.

Screenshot 2024-12-18 at 16 31 50

Comment on lines +150 to +155
windows_appx: {
binary_path: File.join(BUILDS_FOLDER, 'Studio-appx-signed', "Studio #{normalized_version}.0.appx"),
filename_core: 'win32',
extension: 'appx',
name: 'Windows (Appx)'
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need the signed version of the appx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't need ZIP, universal, etc. Let's leave it as is for completeness.

@wojtekn
Copy link
Contributor Author

wojtekn commented Dec 20, 2024

Not sure if this will add the link to the Slack bot, but it would be a nice to have.

It should work automatically, as the code responsible for Slack messages also works around builds object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants