-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: dockerise tldr-lint #309
Conversation
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.
Looks nice, I've got one comment, and a question.
Do we want to publish the linter as a Docker image to GHCR during the publisher workflow? 🤔
Okay, I added a workflow that should push the image when a release is created. However, I have no idea how to test this. Do you have an idea how to test such things? |
Nice changes, I work with Docker and GHCR a lot. Will check out the workflow and get back. |
Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com>
Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com>
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, thanks for your contribution.
Regarding the changes I made,
- I updated the dependabot config for GitHub actions too.
- I changed the base Node version to an LTS one.
- I removed the QEMU action (as
linux/amd
is the default GitHub runner) after testing I noticed the Node base image doesn't support other architectures (i.e.linux/arm
,linux/riscv64
) without performing other changes. - Added labels for the OCI image along with comments to the Dockerfile.
- Removed build and push action as it didn't support a compatible label/annotation to be shown in the package's metadata in GHCR (also it failed actions when executing with workflow dispatch). So IG we can use just a single tag 'latest' for ease of use. [With the current method the package metadata would look something like this.]
- Updated a few dependencies in
test.yml
file and moved the Docker test code as a separate job under it alongside the build job.
Thank you so much for you efforts in improving this PR! |
Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com>
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.
Pushed one small change, but looks great. 👍🏻 Happy for this to be merged.
* add tldr-lint dockerfile * Add docker build workflow * npm install -> npm ci * docker push workflow * fix docker publish wf syntax * insert missing tailing newline * tailing newline * improv: update Docker and CI files Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com> * cleanup: update CI files Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com> * improv: combine publish workflows Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com> * ci: add names for CI workflows --------- Signed-off-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com> Co-authored-by: K.B.Dharun Krishna <kbdharunkrishna@gmail.com> Co-authored-by: Owen Voke <development@voke.dev>
I was getting tired of having to install node in oder to run tldr-lint. I've thus written a Dockerfile which allows users to build a tldr-lint docker image.
I've added a workflow that builds the image in order to check whether or not the build process succeeds.
I've also added documentation in README.md.