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

Packaging improvements #700

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

Conversation

chayleaf
Copy link

@chayleaf chayleaf commented Nov 18, 2023

(see #695)

partially fixes #629 (the initial config is built before loading user options, so the initial config still uses convert from PATH)

This fixes what I consider not following the best packaging practices. Namely:

  • State and resources should be decoupled. Currently, they aren't - the package expects data and config in its directory, even if those are symlinks. Best practice is for any paths to runtime state to be completely decoupled from whatever is in /usr.
  • Config path should be configurable. This ensures testing is easy without editing /etc, you can even run multiple instances of the software with different options. Currently it's hacked upon with config symlink, which is a very bad solution.
  • The app should work fine when started with an empty state directory. Currently, it expects preview/default.jpg to exist there.

@chayleaf chayleaf force-pushed the packaging-improvements branch 9 times, most recently from c1aee70 to bf41a95 Compare November 18, 2023 18:16
@chayleaf
Copy link
Author

chayleaf commented Nov 18, 2023

I've now verified that this works (except the Debian package) and am running this version

@chayleaf chayleaf force-pushed the packaging-improvements branch 5 times, most recently from 1b964b8 to 98eb0f0 Compare November 19, 2023 10:21
@chayleaf
Copy link
Author

tests/lints pass on all commits now, and I've separated the changes into commits a bit better

@chayleaf chayleaf force-pushed the packaging-improvements branch from 98eb0f0 to 6a8b221 Compare November 20, 2023 09:22
@sbs20
Copy link
Owner

sbs20 commented Nov 20, 2023

Thanks for this! There's a lot to review here. I will get to it when I can.

@chayleaf
Copy link
Author

@sbs20 any updates?

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.

convert binary from config isn't being used
2 participants