-
-
Notifications
You must be signed in to change notification settings - Fork 349
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
CI/Linux: Build/publish Docker images used to create AppImages #697
CI/Linux: Build/publish Docker images used to create AppImages #697
Conversation
Fixes: - Forward the $BUILD_DIR variable in $CMAKE_BUILD_OPTIONS. - Add the forgotten '-n' command-line option in 'getopts'. - Update the formatting used to document command-line options.
I've tested the resulting AppImages using
I've also tested both AppImages on my Arch Linux machine, and they seem to work as expected. |
This follows what was done in commit 51ba67b, but for AppImages.
8675539
to
da974a8
Compare
I'll be testing this later today. 👍 |
@nuttyartist Gentle ping. Were you able to test this yet? I was hoping to merge this soon, so we can do a minor release to fix the broken AppImages of v2.3.0. I have the work for the RPM packages ready as well. I'm just waiting for this to get merged first. |
Sorry about that! I’m currently on a holiday, so I’ll try to test it right after. If in your testing this seems to fix the issue then feel free to merge. Did I set you as a collaborator for the notes foss website repo? If so, you could update the links. |
That's no problem. :)
It definitely does, but one extra confirmation is always good.
I believe you did, yes. Anyway, I think I'll wait a few more days for you to test this. If you're still not able to until then, I'll merge this one and the next RPM pull request (I've also tested it). And after that, I'll prepare a new minor release. Would that be acceptable for you? |
I can test it in 5 days, if you don't mind waiting (I forgot my Linux USB stick at home, I believe). |
Sure, no problem. |
Gentle ping. Are you able to test this today? I want to hopefully merge this and the RPM pull request this weekend. |
Yes, thanks for reminding. I will test this today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works.
Still had to add -platform xcb
to the commandline options because of #633, though. As Wayland is becoming the default on many desktop environments, that issue might get more attention.
LGTM, nice changes.
Thank you both for testing! |
This does the exact same as #695 did, but for AppImages now.
I understand this PR introduces a lot of changes, and it's always annoying to review these. So, IMO simply running the resulting AppImages on your Linux distro and making sure they work is a good enough of a "review" for me...
Fixes #692