Update global.json to dotnet 8

CompositionalIT / farmer

Repeatable Azure deployments with ARM templates - made easy!

https://compositionalit.github.io/farmer

MIT License

514 stars 156 forks source link

Update global.json to dotnet 8 #1084

Closed ninjarobot closed 6 months ago

ninjarobot commented 7 months ago

The changes in this PR are as follows:

Updates SDK to dotnet 8.0.100

I have read the contributing guidelines and have completed the following:

[ ] Tested my code end-to-end against a live Azure subscription.
[ ] Updated the documentation in the docs folder for the affected changes.
[ ] Written unit tests against the modified code that I have made.
[ ] Updated the release notes with a new entry for this PR.
[ ] Checked the coding standards outlined in the contributions guide and ensured my code adheres to them.

If I haven't completed any of the tasks above, I include the reasons why here:

SDK update only.

ninjarobot commented 7 months ago

@isaacabraham @martinbryant @OutOfScopeia does anyone have any concerns about updating Farmer to the dotnet 8.0 SDK? This shows the changes - just requires fixing some unused discards in pattern matching.

This does not use newer language features yet so that IDEs and editors can catch up.

ninjarobot commented 7 months ago

@isaacabraham @martinbryant @OutOfScopeia does anyone have any concerns about updating Farmer to the dotnet 8.0 SDK? This shows the changes - just requires fixing some unused discards in pattern matching.

This does not use newer language features yet so that IDEs and editors can catch up.

Seems that VS, VSCode, and Rider have all caught up and have dotnet 8 support.

BrianVallelunga commented 6 months ago

I was just trying to compile on .NET 8 and running into some issues. Thanks for doing this. Hopefully it will be merged soon.

ninjarobot commented 6 months ago

I was just trying to compile on .NET 8 and running into some issues. Thanks for doing this. Hopefully it will be merged soon.

It's the LTS release and tooling has caught up, so if contributors like you are ready for it to upgrade, let's do it. Merging.

BrianVallelunga commented 6 months ago

This pull request resolved the basic .NET 8 upgrade, but because TreatWarningsAsErrors is set to true for this project, I'm still getting build failures due to FS3560 (This copy-and-update record expression changes all fields of record type...)

That leaves us with two options:

1) Exclude the warnings 2) Change the code to remove the unnecessary "with" statement

I'm happy to submit a pull request for either option.

ninjarobot commented 6 months ago

I'd prefer the second option rather than ignoring the warnings. Thanks for fixing this for dotnet 8!