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

Add Docker image for pre-commit #34

Merged
merged 1 commit into from
Aug 20, 2021
Merged

Add Docker image for pre-commit #34

merged 1 commit into from
Aug 20, 2021

Conversation

rally-carlos
Copy link

Is making actionlint available as a Docker image a possibility?

This PR illustrates what the implementation could look like. An account on a Docker image repository is required.

@rhysd
Copy link
Owner

rhysd commented Aug 18, 2021

I think using single binary would be better than using Docker for the following reasons:

  • Overhead is larger. Running container costs more than running a single executable. The Docker image includes entire Go toolchain so it would also require much storage sorry this is not true thanks to Docker's multi-stage builds
  • shellcheck and pyflakes integrations are not available. To enable them, we also need to install them in the image. So maintaining the image is a bit harder

I think, it is easiest way for tools to download executable by using the donwload script and execute the downloaded binary directly.

@rhysd
Copy link
Owner

rhysd commented Aug 18, 2021

OK, I changed my mind. I'll consider to prepare some Docker image for actionlint. I think it is useful for those who don't want to install dependencies like shellcheck or pyflake locally. I can't merge this branch strait forward since I need to investigate how to make a smaller image and add tests for the image.

After the image is prepared, would you rebase this branch onto the latest and add the pre-commit configuration?

@drts01 drts01 force-pushed the docker branch 4 times, most recently from e865736 to 912f429 Compare August 18, 2021 15:07
@drts01
Copy link
Contributor

drts01 commented Aug 18, 2021

I have pushed a Dockerfile to include the dependencies with Alpine Linux as the base image. To make the image smaller, may not be worth the ROI, IMO.

I have also included a test to verify the container can build and run. Does this cover what you are looking for?

P.S. FWIW, I got the Docker idea from hadolint pre-commit implementation. 😄

@drts01 drts01 force-pushed the docker branch 3 times, most recently from 9491b44 to f978589 Compare August 18, 2021 15:28
@rhysd
Copy link
Owner

rhysd commented Aug 19, 2021

Thank you for the update. Looks good. Would you wait for a few days? I'm now improving type checking in other branch. After finishing the task, let me try this branch.

@rhysd
Copy link
Owner

rhysd commented Aug 20, 2021

I confirmed it worked. I released the first version manually: https://hub.docker.com/repository/docker/rhysd/actionlint

I'll add some commits for follow up.

@rhysd rhysd merged commit d935dc6 into rhysd:main Aug 20, 2021
@rhysd
Copy link
Owner

rhysd commented Aug 20, 2021

Merged 👍 Thank you for the additions.

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.

3 participants