-
Notifications
You must be signed in to change notification settings - Fork 38
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 Support with docker-compose.yml #74
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the documentation instructions and Docker configuration. The README now uses updated markdown headers for clearer sectioning and switches the command for live preview from a direct Docker run command to using Docker Compose ( Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docker-compose.yml (1)
1-11
: Service Definition is Correct with a Suggestion for Enhanced Resilience.
The newsphinx
service is defined correctly using thecryptomator_docs_image
, with appropriate port mapping, volume mounts, and command for runningsphinx-autobuild
. For added robustness, consider optionally adding a restart policy (for example,restart: unless-stopped
) and, if desired, a version key at the top of the file.README.md (2)
40-44
: Build Site Command Inconsistency Detected.
While the live preview section has been updated to use Docker Compose, the "Build site" instructions still rely on adocker run
command. Consider updating these instructions to use Docker Compose for consistency, or add a note clarifying why the two commands differ.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
42-42: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
33-34
: Specify Language in Fenced Code Blocks.
To comply with markdownlint rules (MD040), please specify a language (for example,bash
) in the fenced code blocks. This small change will improve documentation clarity.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
33-33: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md
(1 hunks)docker-compose.yml
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
33-33: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (1)
README.md (1)
31-39
: Live Preview Instructions Using Docker Compose Look Great.
The updated instructions now clearly indicate that users should rundocker compose up -d
for live preview. This aligns with the new service indocker-compose.yml
and significantly simplifies the process.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
33-33: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
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.
With the requested changes, we do build the image in the docker compose file. Therefore remove the hole docker install (should be clear how to install docker), build section, only mention to run the docker compose command.
@@ -0,0 +1,10 @@ | |||
services: | |||
sphinx: |
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.
sphinx: | |
cryptomator-docs: |
@@ -0,0 +1,10 @@ | |||
services: | |||
sphinx: | |||
image: cryptomator_docs_image |
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.
image: cryptomator_docs_image | |
build: . |
|
||
``` | ||
docker run -p 8000:8000 -v $(pwd)/source:/source -v $(pwd)/build:/build cryptomator_docs_image sphinx-autobuild -b html /source /build/html --host 0.0.0.0 | ||
docker compose up -d |
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.
docker compose up -d | |
docker compose up |
command: > | ||
sphinx-autobuild -b html /source /build/html --host 0.0.0.0 |
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.
When using >
in yaml files, you can add line breaks that will be removed when parsing.
Ideal to make long commands like this with many parameters more readable.
Changes
Why this change?