search

christianspecht / scm-backup

Makes offline backups of your cloud hosted source code repositories
https://scm-backup.org/
GNU General Public License v3.0
61 stars 20 forks source link

update to .NET 8.0 #77

Closed lessismore-sparkvision closed 4 months ago

lessismore-sparkvision commented 5 months ago

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

christianspecht commented 5 months ago

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 commented 5 months ago

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.

lessismore-sparkvision commented 5 months ago
  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??
christianspecht commented 5 months ago

  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.

lessismore-sparkvision commented 4 months ago

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.