Caliburn-Micro / Caliburn.Micro

A small, yet powerful framework, designed for building applications across all XAML platforms. Its strong support for MV* patterns will enable you to build your solution quickly, without the need to sacrifice code quality or testability.
http://caliburnmicro.com/
MIT License
2.79k stars 776 forks source link

Why are there breaking changes in the 4.0.210 patch release? #819

Open mgroetan opened 2 years ago

mgroetan commented 2 years ago

There is at least one breaking change in this patch(!) release, where the "BootstrapperBase.DisplayRootViewFor" method has been (correctly) renamed to "DisplayRootViewForAsync". Such a change belongs in a major version update, though, and neither is it mentioned in the release notes.

I also note that the .NET Framework requirement has been changed from 4.6.1 to 4.6.2, most likely due to #802. Isn't this too a potential breaking change? NuGet-updating tools like "dotnet-outdated" will now fail to detect an applicable update, due to the way that tool treats framework dependencies (if you're a bit slow on the uptake, and still have some .NET Framework 4.6.1 apps lying about...).

Judging by the release notes, you've also added .NET 6 support. Isn't this also something that would normally warrant a major version release? Just asking for a friend...

vb2ae commented 2 years ago

Thank you for your feedback.

When we released Caliburn.Micro 4 the DisplayRootViewFor was an async function. I just renamed it to keep with async conventions. It is a best practice to await async functions

As for dropping support for 4.6.1 that was to keep in line with Microsoft's supported .net framework versions. I am sure there are plenty of apps that use .net framework 4.6.1 and older. Keep in mind apps using .net framework 4.6.1 and earlier still can use Caliburn.Micro 4.0.173 or earlier

  NET Framework 4.5.2, 4.6, and 4.61 retire on April 26, 2022. These specific releases were previously signed using
  Secure Hash Algorithm (SHA-1) certificates. This algorithm is no longer deemed secure. 

image

https://docs.microsoft.com/en-us/lifecycle/products/microsoft-net-framework

https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core#:~:text=Out%20of%20support%20versions%20%20%20%20Version,August%2021%2C%202021%20%203%20more%20rows%20

I do need to drop support for .net 5 and .net core 3.1 will be out of support in December 2022

Caliburn Micro 5 will include support WinUI, Maui, and AvaloniaUI

mgroetan commented 2 years ago

Thanks for the reply. I appreciate all the good work going into the project, and I don’t have any objections to any of the actually changes as such.

My concern was merely the fact that breaking changes like these do not seem to warrant more than a patch release from you. When updating a NuGet package, you normally don’t expect any breaking changes unless it’s a major version. What I’m trying to achieve by my question is to hear the rationale for NOT making it a major version upgrade. I DO realize that there’s probably parallel ongoing work with the next major version, so having a preemptive one might not fit into into the project plans. But maybe then the breaking changes could have waited for it, If they don’t fix any critical issues?

iberisoft commented 2 years ago

Dear @vb2ae,

Please tell me how to modify the old code broken after updating the NuGet package:

protected override void OnStartup(object sender, StartupEventArgs e)
{
    DisplayRootViewFor<ShellViewModel>();
}

We usually call DisplayRootViewFor in the sync OnStartup. Unfortunately, there is no OnStartupAsync to await DisplayRootViewForAsync. Should I just use the following line?

    DisplayRootViewForAsync<ShellViewModel>().Wait();

BTW, the current documentation still refers to DisplayRootViewFor, not DisplayRootViewForAsync, see https://caliburnmicro.com/documentation/configuration.

kornman00 commented 2 years ago

I'd like to echo iberisoft's comment, as I made the coding-after-midnight mistake of just changing a project's existing code to "async void", which the caller won't wait on: https://github.com/tgjones/gemini/pull/345. Changes worked well enough when running the project's demo at the time, but it's not the right fix.

That change should have also corrected all of the samples. E.g. this still uses the now removed API: https://github.com/Caliburn-Micro/Caliburn.Micro/blob/master/samples/scenarios/Scenario.Functional.App/Bootstrapper.cs#L26

iberisoft commented 2 years ago

Thank you @kornman00, protected override async void OnStartup(object sender, StartupEventArgs e) is a solution.

vb2ae commented 2 years ago

@kornman00 will work on updating the samples