Skip to content

Conversation

@teo-tsirpanis
Copy link
Contributor

A continuation of #8898 (which was subsequently reverted for some reason) but on the entire repository, this PR removes references to NuGet packages from the .NET Standard 1.x era, reducing the amount of transitive dependencies of projects. I also updated two packages to versions that bring less of them, and a notice to Versions.props that discourages adding more of these legacy packages.

And add a note that warns against adding them back.
This includes the file `coreclr_utilities.fs` file, which does not seem to be needed since .NET Core 2.0.
Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

I think these were all necessary when we still built netstandard 1.6. But probably serve no useful purpose now.

Thanks

@teo-tsirpanis
Copy link
Contributor Author

CI currently fails because of some version conflicts caused by the removed packages. They will be fixed by updating dependencies to versions that target .NET Standard 2.0 which I will do later (couldn't catch it earlier, building fails on my machine with compiler errors).

@vzarytovskii vzarytovskii merged commit a6c6db6 into dotnet:main Sep 16, 2022
@teo-tsirpanis teo-tsirpanis deleted the package-cleanup branch September 16, 2022 10:57
@vzarytovskii
Copy link
Member

vzarytovskii commented Sep 19, 2022

We will likely have to roll this one back, it causes build issues when merging to 17.4

@teo-tsirpanis
Copy link
Contributor Author

What fails? Is there a log publicly available?

@vzarytovskii
Copy link
Member

vzarytovskii commented Sep 19, 2022

@teo-tsirpanis

What fails? Is there a log publicly available?

Yeah, this is the merge PR:
#13917
Here's run: https://dev.azure.com/dnceng-public/public/_build/results?buildId=20083&view=results

I will need to take a closer look at it though to be sure if this PR is the cause.
I think our packages diverge in main and 17.4.

@teo-tsirpanis
Copy link
Contributor Author

I don't think this PR is to blame. That PR fails because the property $(MicrosoftCodeAnalysisWorkspacesCommonVersion) is not defined.

@vzarytovskii
Copy link
Member

I don't think this PR is to blame. That PR fails because the property $(MicrosoftCodeAnalysisWorkspacesCommonVersion) is not defined.

Yeah, it seems just a weird merge resolution across multiple PRs

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.

3 participants