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

V15: Update to dotnet 9 #16625

Merged
merged 13 commits into from
Jul 1, 2024
Merged

V15: Update to dotnet 9 #16625

merged 13 commits into from
Jul 1, 2024

Conversation

Zeegaan
Copy link
Member

@Zeegaan Zeegaan commented Jun 19, 2024

Notes

  • Had to rename TrimStart & TrimEnd, as they are now implemented by Microsoft, and the behavior differs from ours.
    • The Umbraco versions are now named TrimStartExact and TrimEndExact
  • Updates dotnet verison to dotnet9
  • Updates nuget packages to their respective dotnet 9 version
  • Had to update Umbraco.Code to 2.2 aswell, as it used the Microsoft.CodeAnalysis package, and thus we couldn't update that.

How to test

Ensure both templates & source code work with dotnet 9

  • Download dotnet 9 preview here: https://dotnet.microsoft.com/en-us/download/dotnet/9.0
  • In the root of the project, run dotnet pack -o nuget
  • Go to the nuget folder cd nuget
  • Copy the path of that folder, and then run dotnet nuget add source [PATH TO NUGET FOLDER HERE]
  • Install the templates, take note of the version of the templates nupkg in the nuget folder dotnet new install Umbraco.Templates::[version of templates here]
  • Create a new umbraco project with the new template dotnet new umbraco -n "MyTestProject"
  • Run the project and assert it works 👍

Copy link
Member

@elit0451 elit0451 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you have a look at the pipelines and update the <TargetFramework> to net9.0 in the csproj's of our dotnet new templates? And the .template.config/template.json has also mentions of net8.0

Directory.Build.props Outdated Show resolved Hide resolved
Copy link
Contributor

@ronaldbarendse ronaldbarendse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a quick look and added my comments/suggestions 😄

Seems like we can actually do more cleaning up of the StringExtensions class, as there's a lot of unused methods or ones that are very specific to our own codebase and could therefore be changed to internal...

build/azure-pipelines.yml Outdated Show resolved Hide resolved
global.json Outdated Show resolved Hide resolved
@Zeegaan
Copy link
Member Author

Zeegaan commented Jun 26, 2024

@ronaldbarendse Cleanup sounds great, but is out of scope for this PR IMO, we can always make a separate one with cleanups 👍

Copy link
Member

@elit0451 elit0451 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and tests out good 💪 Merging

@elit0451 elit0451 merged commit a5b9fd1 into v15/dev Jul 1, 2024
13 of 15 checks passed
@elit0451 elit0451 deleted the v15/feature/update-to-dotnet-9 branch July 1, 2024 07:17
@vsilvar
Copy link
Contributor

vsilvar commented Nov 10, 2024

@Zeegaan Please consider documenting the renaming of the TrimStart and TrimEnd functions as a breaking change.
As this is actually quite a tricky change (outside of Umbraco's control of course), because any consumer currently using the extension methods, will now start using the new Microsoft implementation without their knowledge, and as the behavior differs, this will most likely result in bugs.

FYI @nul800sebastiaan

@nul800sebastiaan
Copy link
Member

It's documented
image

@vsilvar
Copy link
Contributor

vsilvar commented Nov 11, 2024

Well, technically yes, but not really.
It's not directly mentioned in the Umbraco 15 blog post nor in breaking changes documentation.
And even in the Github release notes, it just mentions this PR as: Update to dotnet 9

Not sure if my description of the issue was clear, but just by updating Umbraco, and not changing any code (not even usings):

"FooBarApiOptions".TrimEnd("Options")

Previous result: FooBarApi
New result: FooBarA

@nul800sebastiaan
Copy link
Member

Yikes! I see that .NET 9 seems to split it into chars that need to be trimmed from the end, which is why that p and i from Api are also removed. A much more significant change than I thought it would be.

I'd say this is a good suggestion for both the docs (@sofietoft) and the blog post for the final release (@bergmania) 👍

@ronaldbarendse
Copy link
Contributor

@bergmania @nul800sebastiaan I just updated to the final .NET 9 GA and noticed the overloads with ReadOnlySpan<char> have been removed: https://learn.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/9.0/string-trim.

I'd suggest renaming our extension methods back to TrimStart() and TrimEnd() to avoid unnecessary breaking changes (at least when going from stable to stable versions)!

@nul800sebastiaan
Copy link
Member

This has been reverted here: b9b7cb6

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.

6 participants