-
-
Notifications
You must be signed in to change notification settings - Fork 123
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
add appimage support to packaging #1270
Conversation
Apparently, running AppImage's (like the appimagetool AppImage) would require some changes to the CI. I'm looking into it now. I'm probably going to end up just using the prebuilt binaries instead. |
Changed name of AppImage to confirm with the naming scheme of other artifacts.
Use a underscore instead of hyphen
Probably something to note in a separate issue, but packaging the entire project once as a jpackage app-image and then doing separate packaging off of that would significantly improve CI time. |
@woodser Can I get a review on this? |
@Jabster28 Mind reviewing this? :) I'm not running Linux at the moment. |
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.
Yep works on my machine. A few pedantic notes about the .desktop
file, but it'd be fine to merge as-is. I'll let you know when the sample trade completes.
more descriptive comment add categories add attributes specific to `AppImage`
Everything should be up to date with your suggestions. |
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.
LGTM. @woodser you might want to check the changes to the gradle for formatting etc but everything functionally looks perfect.
Hey, @woodser, is there a problem with the Gradle file formatting or is this good to merge? |
Hi @TheTollingBell, please rebase on master. |
@woodser Rebased and added AppImage artifact |
Thanks. |
@TheTollingBell There's still a merge conflict with master. Once that's resolved I can merge this. Edit: Nevermind, I can merge by squashing your commits :) |
@TheTollingBell Actually there is a merge commit which has conflicts with master. Please do squash your commits to a single commit and rebase on master. Thanks! |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Nevermind was able to work through it. :) |
This resolves issue #1222.
Potential problems:
Haveno.desktop
file to see if it fits with the projects theme.