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 the projects to .NET 8, drop support for .Net Framework, clean up unreachable code. #1573

Closed
wants to merge 6 commits into from
Closed

Conversation

rakitaj
Copy link

@rakitaj rakitaj commented Apr 8, 2024

My attempt at dropping support for .Net Framework and supporting .NET 8.

I did these changes mechanically as possible, and would love discussing if there is a desire for this? I'm happy to make any/all changes the team wants.

My goal here is using .NET 8 to support all OSes and making it easy to use the dotnet tool version of this repo.

  1. Removed net472 from all <TagetFramework> and <TargetFrameworks> elements.
  2. Removed all conditional MSBuild for .Net Framework.
  3. Moved from <TagetFrameworks> and <TargetFramework> because this only targets only framework now.
  4. Remove all references to net472 build artifacts.
  5. Remove and (hopefully) improve the code involved with conditional compilation. I got rid of most of the conditional mechanically - that is I just removed the stuff inside if NETFRAMEWORK. In a few cases I tried to chase down the resulting unreachable code this caused and remove it from the project as well.

The two spots I am the most unsure of my changes are removing support for the OsBroker, which looked like it only worked under .Net Framework and Windows, and removing the interactive auth window.

I did not clean up the Auth options enum because the values had assigned numbers and I didn't want to affect anything which relies on the already assigned numbers.

rakitaj and others added 6 commits March 27, 2024 19:09
1. Get rid of all code inside `if NETFRAMEWORK` conditional compilation
   directives.
2. Attempt to clean up the resulting unreachable code. Anecdotally most
   of it was the OsBroker and interactive window authentication code.

This cleanup was done more aggressively than usual to establish a
starting point for a potential port to .NET along with a drop of .Net
Framework support.
Migrate .NET tool from using ESRP to using the Sign CLI tool for signing. This
tool is a fork of [1] that was set up to support Trusted Signing (previously
known as Azure Code Signing).

1: https://github.com/dotnet/sign
Remove ESRP-related scripts, as we are no longer using this tool for signing.
@rakitaj rakitaj mentioned this pull request Apr 8, 2024
@mjcheetham
Copy link
Collaborator

This PR is a duplicate of the effort in #1418

We are unable to fully drop .NET Framework at this time due to a courtesy to Visual Studio who bundles the same GCM across all their supported versions, and they need older VSs and GCMs to work on Windows 8.1 until 2029(!) 😢

@rakitaj
Copy link
Author

rakitaj commented Apr 15, 2024

Thanks for pointing that out, I didn't see there is a PR already open. I haven't made many open source contributions before, is closing my PR the right move? (I'm happy to do that)

Wow 2029! I'll poke around the issues and see if there is anything else I can help with.

What do you think about upgrading the project to .NET 8 from 7 while keeping everything else the same? I can put together a proof of concept PR for that.

@rakitaj rakitaj closed this May 3, 2024
@rakitaj
Copy link
Author

rakitaj commented May 3, 2024

Hey @mjcheetham I'm closing this PR. I think that's the right move in this case, if you think otherwise just let me know?

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.

4 participants