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

Convert project to dotnet 8 and add github actions #6

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

kikmon
Copy link

@kikmon kikmon commented Jan 29, 2024

Thank you for making this very useful tool
I hope you'll find this PR useful

Converted project to dotnet 8
Added github actions for basic CI

@schellingb
Copy link
Owner

Hi there
First of all, I very much appreciate the time spent on making this PR. I'm very happy that you looked into the code and want to contribute. But sadly I cannot accept this for my project without a good reason. It is important to me that software I maintain runs easily on as many systems as possible. As far as I understand dotnet8 would make RDBed not run out-of-the-box on many Windows versions without the user installing the correct dotnet runtime. I also tried RDBed once under Linux with Mono and it worked successfully. I'm not sure if this would still be the case if we were to upgrade the dotnet version.

Personally I think every additional dependency to a piece of software needs to be warranted with good technical reasons. If for example Windows 11 (or 12) were to drop compatibility with .NET Framework 4.5 we would have a good reason, but as far as I know that is not the case. Another issue with newer dotnet versions is that I can't build or test it because I use Visual Studio 2013.

Is there anything that dotnet8 fixes what is currently broken?

I do like adding github actions, but that alone does not warrant upgrading the runtime, especially for a project with so few commits.

@kikmon
Copy link
Author

kikmon commented Jan 30, 2024

No problem :)
The big advantage of Dotnet 6+is to be able to run natively on Linux and Mac without the whole Mono layer.
As far as I know, the runtime can be packaged with the exe, using the dotnet publish commands in the CI
If you prefer staying with DotNet 4.x, but are interested in the the CI, maybe I can isolate that part and create a new PR ?

@schellingb
Copy link
Owner

That is interesting with running natively on non-Windows operating systems. I did not know this.
Do you know how big such a EXE with a packaged runtime becomes? It would be a bit silly if it were absurdly huge...
And I assume GitHub CI could be used to do the whole packaging for Win/Mac/Linux?

I could maybe see us keeping the code compatible with both 4.5 and something newer so we can keep the current 200 kb EXE for Windows and then maybe something bigger built by CI that runs natively on other OSes. And also so I can keep using Visual Studio 2013 :-)

@kikmon
Copy link
Author

kikmon commented Jan 31, 2024

No idea how big the package gets. It believe it will depend on the actual dependencies of the app.
For reference, the zipped binary generated by the CI is 382 KB
I have no idea if the runtime can be added on Linux or mac, but the application binary itself is independent from the platform
I guess more tests are needed :)
I have never tried to maintain both version at the same time, but it should be doable with a couple if #if here and there.
Dotnet 4.8 seems to be the last one of the 4.x family, and microsoft is putting its energy in the new multiplatfom versions, so dotnet 4.x will suffer the same fate as 3.5 at some point.
Since you have an attachment to VS 2013, I think it important to support both :)
Also, in the PR I targeted dotnet8-windows specifically because I didn't realize you wanted to be able to run the tool on linux and mac.
I'll have to see if we can still run the UI on Linux

@schellingb
Copy link
Owner

I tried to run the output of the GitHub Action on your repository but all I get is a popup that says this:

You must install .NET Desktop Runtime to run this application.

Architecture: x64
App host version: 8.0.1

Learn more:
https://aka.ms/dotnet/app-launch-failed

This is what I want to avoid by targeting an older, wider supported runtime. Also generally I like to keep distribution light not only on file size but also on the number of files. Currently we have a single EXE file that runs on Windows 8 and upwards with no need to install anything. It runs on Windows XP to Windows 7 by installing the dotnet 4.5 runtime and on Linux by installing Mono and Mono-Winforms. If we can run on Windows 7 and older without the need to install anything and on Linux and Mac also with a lighter dependency than Mono, then that would be a win, if it can be achieved without bloating the download too much.

@kikmon
Copy link
Author

kikmon commented Jan 31, 2024

Indeed, the current CI doesn't publish the app with dependencies included, (but it seems to be possible), this is why it's asking you to install the runtime.
That being said, winforms is not supported on non-windows platforms (need to switch to another framework for that), so dotnet 5+ is a no-go for this scenario.
Looks like staying on DotNet 4 is the way to go for now :)

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.

None yet

2 participants