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

[[FEAT]] Add initial docker support #3610

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oliv3r
Copy link

@oliv3r oliv3r commented Feb 26, 2022

This Pull Request adds support for creating containers, such as the famous Docker containers.

Docker containers for users on the docker hub are useful for multiple reasons. One, it would be the 'official' container from jshint, not some (outdated/vulnerable) 3rd party container. Secondly, it would allow pipelines to use the Linter step in a pipeline to use the image container. Thirdly, the container adds an additional layer of testing, as part of its creation, it runs all the tests as well.

Currently, this commit does not yet push to docker hub from circle-ci, but that should be a trivial next step, though some logic with tags probably is desired.

      - run:
          name: Log in to docker hub
          command: echo "${CI_REGISTRY_PASSWORD}" | docker login --username "${REGISTRY_USER}" --password-stdin "${REGISTRY}"
      - run:
          name: Push to docker hub
          command: docker push '${REGISTRY}/jshint:latest'

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
@oliv3r
Copy link
Author

oliv3r commented Feb 26, 2022

Two notes, I did not manage to get the browser test to work, so that's an open issue; due to the GUI-ness needs, and my serious lack of understanding of the test suite.

Also, the 262 test failed since today, after rebasing. I don't see a test result for the latest commits/tag (not on the coverage page).

Secondly, the docker build step in cricle-ci; not sure if that'll work; I have never worked with circle-ci before. It seems to work, yet fails mid-way. Could be output related and the above mentioned test-262 that fails locally as well.

Having a local Containerfile allows anybody to build JSHint containers.
Better yet, it allows JSHint to supply official Docker containers for
users to use, rather then 'random containers from the internet'.

Note, that Containerfile is a modern way to signify agnostic Containers.
Internally, they are 100% pure Dockerfile's but for wider acceptance and
inclusion, Containerfile is the modern take.

The `--file Containerfile` argument might be needed for whichever
builder is used.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
@jugglinmike
Copy link
Member

Thanks for your interest in JSHint!

One, it would be the 'official' container from jshint, not some (outdated/vulnerable) 3rd party container. Secondly, it would allow pipelines to use the Linter step in a pipeline to use the image container.

Docker has been quite popular for many years, but this is the first time I've heard about any interest for an "official" image. I assumed that folks have been satisfied with building their own since it really only takes one build layer on top of the Node.js image.

If anyone out there would find this helpful, I'd appreciate if you would use GitHub's "reaction" feature or (better yet) describe your use case in a comment below.

Thirdly, the container adds an additional layer of testing, as part of its creation, it runs all the tests as well.

Can you say a bit about how running the tests in this environment would be helpful?

@oliv3r
Copy link
Author

oliv3r commented Mar 5, 2022

Hey @jugglinmike

If anyone out there would find this helpful, I'd appreciate if you would use GitHub's "reaction" feature
I think it's great to hold a 'poll' to get user interest.

My personal use cases are two-fold.

For one, I want to be able to quickly run a lint test on a (bunch of) javascript files, without having to create dockerfiles myself etc. I do the same currently for hadolint, shellcheck, markdownlint, pep8, pylint etc. Linting in that case, is a simple container away, always up to date, due to it being maintained, without worry about a 3rd party doing weird or bad things. Trust and all that.

I have a super simple shellscript in my path actually, that wraps this up, so I can just just do 'shellcheck.sh <file/dir>' and it'll run the linter. So being able to do this with javascript would be super nice.

Secondly, More or less the same, but then from a CI system. Especially in GitLab CI, for each stage, you can define an image to use for linting. So having a dedicated and isolated image, that doesn't need to be maintained is very powerful.

I know github has the megalinter (was it?) but the big downside of that is, a) maintainance; github has to make sure ALL linters included are always up to date, but more importantly, b) all linters run in 1 stage, one after the other. By having individual jobs per linter, this can be heavily parallized. I have a few repo's, where 8 linters are running all at the same time, with as a nice bonus, a pipeline indicator telling me WHICH linter failed, without having dig through logs; which is where case 1 comes in handy again. Much like the circleci output we see in this MR ;)

Mind you, I certainly understand this is not the 'traditional' webdevelopment flow, that most javascript developers would follow (they could of course) where one defines one container, puts everything in it, and deals with that.

Can you say a bit about how running the tests in this environment would be helpful?
I don't think it should replace your excellent test suite that you already have. But the container for one is based on alpine, which I'm not sure is part of your test suite, but more importatantly, the test-suit gets run as part of container creation, so BESIDES your confidence that the test suite passed; the creation of the container will enhance this even, in that we should be very certain, that the created container is of high-quality, without any additional effort or risks.

E.g. pulling in jshint from npm, can still cause some issues if the host is not exactly matching with when the package was uploaded. As there would be no tests run (usually), then there would always be risk on full functionality. Likewise when skipping the tests in container creation. The confidence factor would be slightly lower. Test what you ship, kind of mantra. Most importantly though; it costs almost nothing to run in addition to; so it's almost a free-bee :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants