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

Restore sorting empty target framework last #242

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Jan 6, 2024

Commit 8e85a15 dropped the NuGet.Frameworks dependency and as a result sorting of the target frameworks behaviour changed.

Previously, the NuGetFrameworkSorter would sort the empty target framework last. Since 8e85a15 the empty target framework is sorted first.

This commit restores the old behaviour of sorting the empty target framework last.

Note: Stryker.NET was inadvertently depending on this behaviour, so v6.0.1 broke Stryker.NET. I have fixed the issue in Styker.NET but I think restoring the previous sorting behaviour is a good idea anyway.

Commit 8e85a15 dropped the NuGet.Frameworks dependency and as a result sorting of the target frameworks behaviour changed.

Previously, the `NuGetFrameworkSorter` would sort the empty target framework last. Since 8e85a15 the empty target framework is sorted first.

This commit restores the old behaviour of sorting the empty target framework last.

Note: Stryker.NET was inadvertently depending on this behaviour, so v6.0.1 broke Stryker.NET. I have [fixed the issue in Styker.NET](stryker-mutator/stryker-net#2811) but I think restoring the previous sorting behaviour is a good idea anyway.
@daveaglick
Copy link
Collaborator

Yikes, sorry! And good catch - nice work on bringing back a TargetFrameworkComparer that doesn't depend on NuGet.Frameworks. Merging and deploying a patch now.

@daveaglick daveaglick merged commit fb2449d into phmonte:main Jan 6, 2024
6 checks passed
@0xced 0xced deleted the sort-tfm branch January 6, 2024 16:51
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