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

Enable win-arm64 as known RID for AspNetCore #8173

Closed
wants to merge 1 commit into from

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Aug 14, 2020

No description provided.

@hoyosjs hoyosjs added this to the 5.0.1xx milestone Aug 14, 2020
@hoyosjs hoyosjs self-assigned this Aug 14, 2020
@hoyosjs hoyosjs requested review from sfoslund and wli3 August 14, 2020 20:34
@hoyosjs
Copy link
Member Author

hoyosjs commented Aug 14, 2020

cc @tommcdon

Copy link
Member

@sfoslund sfoslund left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed review, thank you for your patience.

@@ -108,7 +108,7 @@
<Crossgen2SupportedRids Include="linux-musl-x64;linux-x64;win-x64" />

<AspNetCore31RuntimePackRids Include="@(AspNetCore30RuntimePackRids)" />
<AspNetCoreRuntimePackRids Include="@(AspNetCore31RuntimePackRids)" />
<AspNetCoreRuntimePackRids Include="@(AspNetCore31RuntimePackRids);win-arm64" />
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a supported RID for all versions or just 5.0+? Can we create another itemgroup to include this in and follow the pattern defined by AspNetCore30RuntimePackRids and AspNetCore31RuntimePackRids?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for 5.0. Should this be renamed then to AspNetCore5.0RuntimePackRids? It just seemed that the tip has the version-less element so I followed that.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think creating a AspNetCore50RuntimePackRids item group makes sense here.

@marcpopMSFT
Copy link
Member

@hoyosjs are you still working on this pr?

@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 9, 2020

@marcpopMSFT yes, although I was planning to merge it with the ARM64 PR I have coming up for the installers so it can be more easily ported to the release trains. Any opinions on that?

@wli3
Copy link

wli3 commented Sep 10, 2020

What do you mean by "release trains"?

RC2 is closing soon. You need to finish it before 9/24 to catch 5.0 release

@hoyosjs
Copy link
Member Author

hoyosjs commented Sep 10, 2020

Closing in favor of #8470

@hoyosjs hoyosjs closed this Sep 10, 2020
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.

4 participants