scm-backup icon indicating copy to clipboard operation
scm-backup copied to clipboard

update to .NET 8.0

Open lessismore-sparkvision opened this issue 1 year ago • 4 comments

Continued branch net6.0 to net 8.0. Consider to add mailname to settings.yml

lessismore-sparkvision avatar Apr 27 '24 06:04 lessismore-sparkvision

Thanks, looks good!

Two questions:

  1. When I merge this and make a new release, which version number would you give that release?
    On one hand, it's a breaking change - all users who run SCM Backup on an actual machine (and not just via Docker) have to install a new .NET runtime.
    On the other hand, releasing a version 2.0 with no functional changes feels wrong. I think I'll just increase the minor version (so the new release would be v1.8.0), but I'm interested in a second opinion.

  2. You wrote:

    Consider to add mailname to settings.yml

    Can you elaborate? It's probably something in the MailkitEmailSender, but it's years since I last touched this file, and from the top of my head I don't know what mailname is. Thanks

christianspecht avatar Apr 30 '24 20:04 christianspecht

Oh, and the Linux build failed. You only changed the AppVeyor file in your PR, but there's also this GitHub Action.

Strange, though. It looks as if the actual build worked (even though the build script installed .NET Core 3.x on the runner), and the tests are failing because the environment variables are missing. Maybe it's because the variables are defined in my repo and the build is running from your fork, not sure.

christianspecht avatar Apr 30 '24 20:04 christianspecht

  1. Regarding version number - it's in a gray zone for me as well. Following semver.org minor versions should be backward compatible, but this is not. It requires a new runtime. Major versions are for incompatible API changes, but it is not. Personally, I vote for 2.0 with documentation what has changed.
  2. Mailkit has no overload with address only, so used MailboxAddress(string name, string address) with a hardcoded "" for name. My idea was to add name in settings.yml and replace the hardcoded "". (src/ScmBackup/Http/MailKitEmailSender.cs row 25).
  3. I forgot to update github workflow. As you have identified, build script has to be updated to use net8.0 and environmental variables are missing. From the script, it looks like you are using Github secrets to define and set the environmental variables??

lessismore-sparkvision avatar May 02 '24 11:05 lessismore-sparkvision

  1. Still thinking about the version number.

  2. I'm not against providing a name via config, but is that really important for anybody? I wouldn't mind "SCM Backup" as a hardcoded name ;-)

  3. Yes, the environment variables are coming from Github repository secrets. No idea what to do, so an Action triggered by your fork can read them. I guess we have to wait until I merge it and the action runs in my repo. But the Appveyor build worked, so I don't see why the Actions build wouldn't work.

christianspecht avatar May 06 '24 21:05 christianspecht

Sorry for the delay. A lof to do privately...

  1. Your choice.
  2. Will use "SCM Backup" as a hardcoded name.
  3. I agree with your guess.

Will update PR with "SCM backup" suggestion tonight.

lessismore-sparkvision avatar May 13 '24 09:05 lessismore-sparkvision