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

Dockerfile introduced #391

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

Dockerfile introduced #391

wants to merge 3 commits into from

Conversation

typekpb
Copy link

@typekpb typekpb commented Nov 5, 2020

No description provided.

@MikeRalphson
Copy link
Contributor

Thanks for the PR. The Dockerfile looks a little generic to me (though I'm no expert). Wouldn't we need to expose a volume mapping so people can specify the input file and get at the output?

@typekpb
Copy link
Author

typekpb commented Nov 5, 2020

you're right, but usage is kind of case-specific. For example, if I follow the official Example, it could be done like:

docker run -it --rm -v ${PWD}:/usr/src/app/petstore3.json:ro -v ${PWD}/md/:/usr/src/app/md/ widdershins:latest /bin/sh -c "node widdershins /usr/src/app/petstore3.json --search false --language_tabs 'ruby:Ruby' 'python:Python' --summary -o /usr/src/app/md/petstore3.html.md"

@MikeRalphson
Copy link
Contributor

I think we would need an example added to either the README.md, a docs/docker.md file or the wiki...

Can I ask (I know I'm biased) but why use Docker to run node.js programs at all? It's very easy to install node on every platform I've ever tried it on...

@typekpb
Copy link
Author

typekpb commented Nov 5, 2020

README.md file updates provided (with TODOs for dockerhub org/link)

why docker? because then docker is the only dependency we need to install to our documentation pipeline. And widdershins is not the only tool we use. And we don't need to care about the updates as docker pull does the job

@jalaziz
Copy link

jalaziz commented Nov 13, 2020

Thanks for the PR. The Dockerfile looks a little generic to me (though I'm no expert). Wouldn't we need to expose a volume mapping so people can specify the input file and get at the output?

I would discourage specifying a volume in the Dockerfile. It's trivial to document that a volume could be used (via CLI or whatever), but specifying a volume in the Dockerfile itself can result in volume leaking if anyone inherits from this docker image and specifies their own volumes. Docker currently doesn't allow you to remove volumes from base images despite a long running issue asking for that behavior.

@typekpb
Copy link
Author

typekpb commented Nov 13, 2020

@MikeRalphson any chance to get this one processed?

@MikeRalphson
Copy link
Contributor

@typekpb sorry, yes, don't see why not, but do we need a .dockerignore file to prevent copying any existing node_modules or .git directories into the image?

@typekpb
Copy link
Author

typekpb commented Nov 23, 2020

@MikeRalphson .dockerignore introduced

@typekpb
Copy link
Author

typekpb commented Mar 4, 2021

seems to have low prio, right?

@MikeRalphson
Copy link
Contributor

Sorry no, it will be included in the next release, but the features for that are still being worked on.

@adrianlungu
Copy link

Is this still on the roadmap ?

@steveannett
Copy link

@MikeRalphson will this be added as a feature soon? I'd really appreciate having the Dockerfile added as it would make certain build pipelines easier. Thank you for the awesome stuff you've built.

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

Successfully merging this pull request may close these issues.

5 participants