Skip to content

Update ILAsm and ILDasm version to match expected SDK version#36154

Merged
JoeRobich merged 2 commits intomasterfrom
dev/jorobich/update-ilasm-ildasm
Jun 4, 2019
Merged

Update ILAsm and ILDasm version to match expected SDK version#36154
JoeRobich merged 2 commits intomasterfrom
dev/jorobich/update-ilasm-ildasm

Conversation

@JoeRobich
Copy link
Copy Markdown
Member

As pointed out by @AlekseyTs, #36042 should have also bumped the ILAsm and ILDasm version numbers.

Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 2)

Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Should we use a common property shared by all so this doesn't happen again?

@JoeRobich
Copy link
Copy Markdown
Member Author

Should we use a common property shared by all so this doesn't happen again?

@jasonmalinowski The version of ILAsm and ILDasm expected is found by

  1. Check what version of NetCoreApp the new SDK is targeting (in the folder under shared\Microsoft.NETCore.App look in the .version file)
  2. Using the dotnet-coreclr NuGet feed (https://dotnetfeed.blob.core.windows.net/dotnet-coreclr/index.json) find the last published version of ILAsm/ILDasm prior to the NetCoreApp version

@jasonmalinowski
Copy link
Copy Markdown
Member

@JoeRobich That seems...roundabout. Wonder what other options we have? Let's merge this of course if something is broken but wondering how we can decouple things to be less tricky.

@JoeRobich JoeRobich merged commit 968fae5 into master Jun 4, 2019
@JoeRobich JoeRobich deleted the dev/jorobich/update-ilasm-ildasm branch June 7, 2019 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants