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

Remove script for building self-contained package #9891

Merged
merged 7 commits into from Feb 3, 2024

Conversation

maryamariyan
Copy link
Member

@maryamariyan maryamariyan commented Feb 1, 2024

Summary of the changes

  • Removed RuntimeIdentifiers and self contained packaging related content in csproj
  • Set SelfContained to false

Unsure if something else needs to be added,
Also unsure of how best to test the end to end on the extension side.

Related PRs:

@maryamariyan maryamariyan requested a review from a team as a code owner February 1, 2024 21:52
src/Razor/src/rzls/rzls.csproj Outdated Show resolved Hide resolved
src/Razor/src/rzls/rzls.csproj Outdated Show resolved Hide resolved
@maryamariyan maryamariyan requested a review from a team as a code owner February 1, 2024 22:25
- RuntimeIdentifiers should not be needed for framework dependent
- Specifically setting SelfContained to false
- RuntimeIdentifierForPublish using RuntimeIdentifiers should not be used/needed anymore
- Updated ProjectToPublish to happen once per project not N times (per RuntimeIdentifier)
@maryamariyan
Copy link
Member Author

maryamariyan commented Feb 2, 2024

Here's some notes from looking a bit further into our build scripts:

In our publishing code I saw a reference to the comment below by @dsplaisted

Usage:

--- UPDATE

After our offline discussion about platform specific vs agnostic, seems like we still need to be doing platform specific work so, as instructed I just removed the self contained bits and left everything else as-is

Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

Strictly speaking we don't need to specify SelfContained=false, but I have no problem being specific.

seems like we still need to be doing platform specific work

Just my 2c, but even if this wasn't true, I would still be very very strongly in favour of doing any work related to that in a separate PR. Changing multiple variables at the same time just makes testing and validation so much harder. Thankfully the point is moot :)

@maryamariyan maryamariyan merged commit 7325b1c into main Feb 3, 2024
12 checks passed
@maryamariyan maryamariyan deleted the dev/maryamariyan/non-self-contained branch February 3, 2024 01:05
@ghost ghost added this to the Next milestone Feb 3, 2024
@jjonescz jjonescz modified the milestones: Next, 17.10 P2 Feb 27, 2024
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.

None yet

4 participants