search

shymega / RSSingle

Merge multiple feeds (RSS/Atom/JSON) into a single RSS feed.
Apache License 2.0
14 stars 3 forks source link

Add Docker support for RSSingle, and a workflow for builds #5

Closed shymega closed 8 months ago

shymega commented 2 years ago

This commit adds a Dockerfile, .dockerignore, and a GH Action for pushing container images to - and this is by default, other registries can be used - Docker Hub (you need to generate a token and install it in the repo), and GitHub Registry, which doesn't require a manual token.

The workflow won't build until #3 is merged, as it relies on successful builds from that workflow before a container image is pushed. Once we use unit tests, this will ensure container images aren't broken when pushed to a remote registry.

I have marked this PR as draft for that reason.

Signed-off-by: Dom Rodriguez shymega@shymega.org.uk

tanrax commented 1 year ago

@shymega Interesting, why the change?

shymega commented 1 year ago

@tanrax It's mostly a way to use the Docker image in GH Actions with the container YAML key. This means there's no requirement for a separate download of RSSingle.

tanrax commented 1 year ago

Excellent work. I will add some changes separately for your review.

shymega commented 1 year ago

I'd argue that 433e6ca is unnecessary. Because of the way I designed the Dockerfile, it's a "multi-stage build". The first stage is only temporary, and any caches won't be transferred into the app layer. In fact, the only file we're copying is the pyinstaller-generated file. So in the final container image, there is no caching copied over. If you want, I can remove that commit (not revert, but rebase and remove), and then push - and we can mark this PR as ready for review.

I should have made that clearer in the documentation. Sorry about that.

shymega commented 8 months ago

I've just come across this PR again - we could definitely merge, once 433e6ca is removed from the PR, as it doesn't affect the final artefact built.

What do you think?