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 high-level description of docker images and their build process #549

Merged
merged 7 commits into from
Jun 28, 2023

Conversation

VJalili
Copy link
Member

@VJalili VJalili commented Jun 14, 2023

This PR extends the website by adding documentation about the docker images, their relationship, the motivation behind building them into multiple images, and a high-level description of their build, test, and publish process.

The documentation on building, pushing, developing, and updating CI/CD for other container registries will be added in follow-up PRs.

Copy link
Collaborator

@mwalker174 mwalker174 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for putting this together. The docker system has become a bit complex over the years and this adds a lot of clarity as to how the build system works.

I have a few minor suggestions and also it looks like the manual docker publishing section is missing some content.

In addition, the documentation is very technical and I think we want to make it clear that this is intended for developers and not users (who we don't want to scare away ;)). One way to do that would be place the "Docker Images" as a subsection under a "Development" or "For Developers" section. Not sure how that will look visually since there's already a few levels of subsections, so you could try to just add a clear disclaimer to the Overview.

On a related note, I think it would be helpful to have a small section under "Getting Started" about dockers that is intended for users. A quick description of where to find dockers.json and that the images there are updated with workflows and other code.

website/static/img/docker_hierarchy.png Outdated Show resolved Hide resolved
website/docs/docker/images.md Outdated Show resolved Hide resolved
website/docs/docker/deploy/manual.md Outdated Show resolved Hide resolved
website/docs/docker/index.md Outdated Show resolved Hide resolved
@VJalili
Copy link
Member Author

VJalili commented Jun 28, 2023

Thanks for the review, @mwalker174! I implemented the suggested changes; I agree it reads better now; thank you! I would appreciate it if you could re-review it.

Copy link
Collaborator

@mwalker174 mwalker174 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just a couple last comments but on the whole good to go.

website/docs/advanced/docker/images.md Outdated Show resolved Hide resolved
website/docs/advanced/docker/deploy/manual.md Outdated Show resolved Hide resolved
@VJalili VJalili merged commit 0b6e825 into broadinstitute:main Jun 28, 2023
3 checks passed
@VJalili VJalili deleted the docs branch June 28, 2023 19:37
@VJalili
Copy link
Member Author

VJalili commented Jun 28, 2023

Thanks for the review, @mwalker174!

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.

2 participants