feat(docs): Add make serve-docs, fix live reload#2151
feat(docs): Add make serve-docs, fix live reload#2151jgwest merged 1 commit intoargoproj-labs:masterfrom
make serve-docs, fix live reload#2151Conversation
Signed-off-by: Oliver Gondža <ogondza@gmail.com>
📝 WalkthroughWalkthroughThe changes containerize the documentation serving workflow by introducing a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/developer-guide/development.md (1)
148-150: Add a short usage note aftermake serve-docs.Consider documenting the expected URL and runtime prerequisite (Docker/Podman) for first-time contributors.
Proposed doc tweak
```bash make serve-docs
+This requires Docker or Podman. After startup, open
http://localhost:8000.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/developer-guide/development.mdaround lines 148 - 150, Add a short
usage note immediately after the code block that shows the make targetmake serve-docs: state the runtime prerequisite (e.g., "Requires Docker or Podman")
and the expected URL to open after startup (e.g., "Open http://localhost:8000").
Keep the note concise and placed right after the existingmake serve-docs
snippet so first-time contributors see the prerequisite and the URL immediately.</details> </blockquote></details> <details> <summary>docs/requirements.txt (1)</summary><blockquote> `11-12`: **Use `click<8.3` instead of strict pin to allow safe patch updates while avoiding known-bad versions.** Click 8.2.1 works, but versions 8.3.0 and 8.3.1 break mkdocs serve live-reload. A bounded constraint is more resilient than a strict pin for a temporary workaround—it allows 8.2.x patches and keeps the workaround maintainable. The underlying mkdocs issue remains open. <details> <summary>Proposed adjustment</summary> ```diff -# Workaround for https://github.com/mkdocs/mkdocs/issues/4032 -click==8.2.1 +# Workaround for https://github.com/mkdocs/mkdocs/issues/4032 +click<8.3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/requirements.txt` around lines 11 - 12, Replace the strict pin "click==8.2.1" in requirements.txt with a bounded constraint that prevents 8.3.x but allows 8.2.x patches (for example use "click<8.3,>=8.2.0" or at minimum "click<8.3"); update the dependency line so mkdocs avoids the known-bad 8.3.0/8.3.1 releases while still receiving safe 8.2.x patch updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 392-393: The serve-docs Makefile target should fail fast when
CONTAINER_RUNTIME is unset or empty: in the serve-docs recipe (target name
serve-docs) add a short guard that checks the CONTAINER_RUNTIME variable and
exits with a clear error if it's not set (e.g., test -n "$(CONTAINER_RUNTIME)"
|| { echo "CONTAINER_RUNTIME is not set"; exit 1; }) before invoking
$(CONTAINER_RUNTIME) run ... so the recipe aborts with a readable message rather
than attempting to run an empty command.
---
Nitpick comments:
In `@docs/developer-guide/development.md`:
- Around line 148-150: Add a short usage note immediately after the code block
that shows the make target `make serve-docs`: state the runtime prerequisite
(e.g., "Requires Docker or Podman") and the expected URL to open after startup
(e.g., "Open http://localhost:8000"). Keep the note concise and placed right
after the existing `make serve-docs` snippet so first-time contributors see the
prerequisite and the URL immediately.
In `@docs/requirements.txt`:
- Around line 11-12: Replace the strict pin "click==8.2.1" in requirements.txt
with a bounded constraint that prevents 8.3.x but allows 8.2.x patches (for
example use "click<8.3,>=8.2.0" or at minimum "click<8.3"); update the
dependency line so mkdocs avoids the known-bad 8.3.0/8.3.1 releases while still
receiving safe 8.2.x patch updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b709cd5-4355-465d-94cb-ebc4fc3cd319
📒 Files selected for processing (3)
Makefiledocs/developer-guide/development.mddocs/requirements.txt
| serve-docs: ## Serve documentation locally using mkdocs in a container | ||
| $(CONTAINER_RUNTIME) run --rm -it -p 8000:8000 -v $(PWD):/argocd-operator -w /argocd-operator --name argocd-operator-mkdocs registry.access.redhat.com/ubi9/python-311:latest /bin/bash -c "pip install -r docs/requirements.txt && mkdocs serve -a 0.0.0.0:8000" |
There was a problem hiding this comment.
Fail fast in serve-docs when no container runtime is available.
This target can still execute with an empty CONTAINER_RUNTIME, leading to a confusing failure. Add an explicit runtime check in the recipe.
Proposed fix
.PHONY: serve-docs
serve-docs: ## Serve documentation locally using mkdocs in a container
+ `@if` [ -z "$(CONTAINER_RUNTIME)" ]; then \
+ echo "No container runtime found (docker/podman)."; \
+ exit 1; \
+ fi
$(CONTAINER_RUNTIME) run --rm -it -p 8000:8000 -v $(PWD):/argocd-operator -w /argocd-operator --name argocd-operator-mkdocs registry.access.redhat.com/ubi9/python-311:latest /bin/bash -c "pip install -r docs/requirements.txt && mkdocs serve -a 0.0.0.0:8000"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| serve-docs: ## Serve documentation locally using mkdocs in a container | |
| $(CONTAINER_RUNTIME) run --rm -it -p 8000:8000 -v $(PWD):/argocd-operator -w /argocd-operator --name argocd-operator-mkdocs registry.access.redhat.com/ubi9/python-311:latest /bin/bash -c "pip install -r docs/requirements.txt && mkdocs serve -a 0.0.0.0:8000" | |
| serve-docs: ## Serve documentation locally using mkdocs in a container | |
| `@if` [ -z "$(CONTAINER_RUNTIME)" ]; then \ | |
| echo "No container runtime found (docker/podman)."; \ | |
| exit 1; \ | |
| fi | |
| $(CONTAINER_RUNTIME) run --rm -it -p 8000:8000 -v $(PWD):/argocd-operator -w /argocd-operator --name argocd-operator-mkdocs registry.access.redhat.com/ubi9/python-311:latest /bin/bash -c "pip install -r docs/requirements.txt && mkdocs serve -a 0.0.0.0:8000" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 392 - 393, The serve-docs Makefile target should fail
fast when CONTAINER_RUNTIME is unset or empty: in the serve-docs recipe (target
name serve-docs) add a short guard that checks the CONTAINER_RUNTIME variable
and exits with a clear error if it's not set (e.g., test -n
"$(CONTAINER_RUNTIME)" || { echo "CONTAINER_RUNTIME is not set"; exit 1; })
before invoking $(CONTAINER_RUNTIME) run ... so the recipe aborts with a
readable message rather than attempting to run an empty command.
There was a problem hiding this comment.
Ignore, we depend on that in many places of the Makefile anyway
There was a problem hiding this comment.
@olivergondza, understood — the pattern is consistent throughout the Makefile, with $(CONTAINER_RUNTIME) used unguarded in many other targets (e.g., docker-build, bundle-build, util-build). The top-level $(warning ...) already handles the empty-runtime case uniformly. I'll drop this suggestion.
✏️ Learnings added
Learnt from: olivergondza
Repo: argoproj-labs/argocd-operator PR: 2151
File: Makefile:392-393
Timestamp: 2026-04-08T08:37:58.087Z
Learning: In the argocd-operator Makefile, `CONTAINER_RUNTIME` is intentionally used without per-target guards across all container-related targets (docker-build, bundle-build, util-build, serve-docs, etc.). A top-level `$(warning ...)` already handles the empty-runtime case. Do not suggest adding per-target runtime guards as they are inconsistent with the established pattern.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
jgwest
left a comment
jgwest
left a comment
There was a problem hiding this comment.
LGTM, works great, thanks @olivergondza!
For folks on Linux w/ SELinux enabled (e.g. default fedora), they will need to use -v $(PWD):/argocd-operator:z, but that's not something we should include in Makefile.
2 participants
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
clickpipHave you updated the necessary documentation?
Which issue(s) this PR fixes:
N/A
How to test changes / Special notes to the reviewer:
Summary by CodeRabbit
Release Notes
New Features
serve-docsmake target that provisions a containerized environment and serves documentation locally on port 8000.Documentation
Chores