-
Notifications
You must be signed in to change notification settings - Fork 266
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
Added .Net6 target framework #674
Conversation
…upported in latest unit test framework
I have published custom version of this package to nuget as NSubstituteNet6, it can be deprecated when the official package gets .NET 6 support https://www.nuget.org/packages/NSubstituteNet6/ it also fixes the security vulnerabilities mentioned here: #673 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR. LGTM 👍
Will see if @zvirja or @alexandrnikitin have any comments then will merge.
@Havunen Would it be possible to keep It would also help if you could drop a bit more info on reasoning of this PR (except you being generous and help to develop the project 😊). Is it blocking you in certain scenarios? This would also help to understand how soon it should be released. Thank you! |
Ok, I can take a look.
I was just trying to remove known security vulnerabilities from our project and then ended up here 😂 |
added back netstandard 1.3 target framework |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I think we can go ahead and publish a new minor!
@Havunen Thank you for your contribution! I personally wouldn't update |
Revert package version update
@alexandrnikitin ok I reverted that version upgrade, however it sucks big time to start installing dependencies of dependency at application level. Then one day the dependency refactors given dependency away and the application is still stuck with it, because nobody knows why it exists or why it was added and following these dependencies in each version upgrade is non trivial task in larger projects |
Could this PR be merged and new version published to nuget? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
One minor change for our tests project targets.
tests/NSubstitute.Acceptance.Specs/NSubstitute.Acceptance.Specs.csproj
Outdated
Show resolved
Hide resolved
@Havunen I understand your concerns regarding dependency management. It's a hard problem to crack and .NET world is not the worst compared to other ecosystems. I believe it should be on package manager's, IDE's or code analyzer's shoulders to solve it. AFAIK latest Visual Studio or R# can help with unused packages. |
Merged. Thank you for the contribution and review discussion! 🙇 |
Any info when will this be published to nuget? |
Releasing changes from nsubstitute#636 and nsubstitute#674.
@aboryczko This has been published as 4.3.0. Sorry for the delay. |
@dtchepak thanks :) |
Fixes #675
Adds .net6.0 LTS compilation target and updates the package references to latest version
@dtchepak Could you review this PR please :)