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

Update Roslyn based libraries to 4.0.1 and use .NET 6 SDK with multitarget #197

Merged
merged 2 commits into from
Feb 13, 2022

Conversation

AdaskoTheBeAsT
Copy link
Contributor

Another way of doing upgrade in comparison to
#189

@daveaglick
Copy link
Collaborator

Ah, very clever using the StructuredLogger package to provide up-to-date log handling while downgrading the CodeAnalysis packages on .NET Core 3.x. I like where you’re going with this.

At this point, why even bother with the .NET 5/6 targets though? This drops .NET Framework support (which is okay) and .NET 5/6 can correctly consume a .NET Core 3.x package, so do we gain anything with the multi-targeting or would it be sufficient just to take your changes for .NET Core 3.1 and leave it at that for now?

I don’t like the idea of this much conditional on the target, and I have a feeling it’ll just continue to balloon as time goes on, so I don’t want to maintain it in this state. But if only targeting .NET Core 3.1 works with your approach we’ll do that and keep moving forward on that target as long as we can. If not, I’ll merge this PR for one final release before a new major release like what I was working on with a .NET 6 target.

Either way, nice work!

@AdaskoTheBeAsT
Copy link
Contributor Author

At this point, why even bother with the .NET 5/6 targets though? This drops .NET Framework support (which is okay) and .NET 5/6 can correctly consume a .NET Core 3.x package, so do we gain anything with the multi-targeting or would it be sufficient just to take your changes for .NET Core 3.1 and leave it at that for now?

I am not convinced to providing in that moment .net5/6 multitarget or stick with 3.1. As you know much more about project direction please choose :)

@daveaglick
Copy link
Collaborator

Cool - I'm working on this right now, stripped out the .NET 5/6 stuff and focusing on your changes to support .NET Core 3.1. If that works without the 5/6 targets we'll move forward with that approach as a baseline until we really need to update to .NET 6 at some point in the future. Even though we'll still have to drop .NET Framework support (I don't really see a way around that), this at least keeps the .NET Core 3.1 support for folks who need it.

@daveaglick
Copy link
Collaborator

Everything worked great and all tests passed, so I'm going to move forward with this change to keep us on .NET Core 3.1 for now. Very nice work!! Saved me quite a bit of trouble downstream in some other projects for now.

@daveaglick daveaglick merged commit 53b3a18 into phmonte:main Feb 13, 2022
@daveaglick daveaglick mentioned this pull request Nov 29, 2022
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