Documentation changes to address : Maintainer Alignment: Documentatio…#96
Documentation changes to address : Maintainer Alignment: Documentatio…#96oidebrett wants to merge 1 commit into
Conversation
…n preferences and open questions hhftechnology#14
|
@ivobrett is attempting to deploy a commit to the HHF Technologies' projects Team on Vercel. A member of the Team first needs to authorize it. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the CrowdSec Manager documentation with new feature guides for Config Validation, History, Hub Browser, Multi-Host Docker, Simulation Mode, and Terminal. It also updates the backend architecture to reflect the transition to the Gin router, raises the minimum Go version requirement to 1.23, and adjusts the API response format. Feedback focuses on refining the documentation by removing internal notes and meta-comments, ensuring consistent API prefixes in examples, and recommending that Podfile.lock be tracked in version control rather than ignored.
| mobile/ios/App/App.xcworkspace/contents.xcworkspacedata | ||
| mobile/ios/App/Podfile.lock |
There was a problem hiding this comment.
The Podfile.lock file should typically be tracked in version control for application projects. It ensures that all developers and the CI/CD environment use the exact same versions of dependencies, preventing inconsistent behavior across environments caused by version mismatches.
mobile/ios/App/App.xcworkspace/contents.xcworkspacedata
| @@ -0,0 +1,98 @@ | |||
| --- | |||
| title: Multi-Host Docker | |||
| description: Manage multiple Docker hosts (docs say "not available") | |||
There was a problem hiding this comment.
The description contains an internal note '(docs say "not available")' that should be removed for the public-facing documentation.
description: Manage multiple Docker hosts
| ## Related Topics | ||
|
|
||
| - [Configuration](../configuration/index.mdx) | ||
| - [Hosts Management](../features/hosts.mdx) *(if separate hosts feature exists)* |
There was a problem hiding this comment.
The related topic link contains a meta-comment '(if separate hosts feature exists)' which should be removed or resolved before merging.
- [Hosts Management](../features/hosts.mdx)
| - **Secure Connection**: WebSocket-based connection with proper authentication. | ||
|
|
||
| ## API Endpoints | ||
|
|
There was a problem hiding this comment.
For consistency with other feature documentation in this PR (e.g., History, Hub Browser), it should be explicitly mentioned that the terminal endpoint is served under the /api prefix.
The Terminal exposes the following WebSocket endpoint under the `/api` prefix:
|
|
||
| ```javascript | ||
| // Example using JavaScript WebSocket API | ||
| const ws = new WebSocket('ws://localhost:8080/terminal/crowdsec'); |
There was a problem hiding this comment.
The WebSocket URL in this example is missing the /api prefix. Based on the route registration in internal/api/routes.go, the terminal endpoint is served under the /api group.
const ws = new WebSocket('ws://localhost:8080/api/terminal/crowdsec');
|
|
||
| - [Configuration](../configuration/index.mdx) | ||
| - [Logs Monitoring](../features/logs-monitoring.mdx) | ||
| - [Multi-Host Docker](../features/multi-host.mdx) *(if applicable)* No newline at end of file |
There was a problem hiding this comment.
The related topic link contains a meta-comment '(if applicable)' which should be removed to maintain a professional documentation style.
- [Multi-Host Docker](../features/multi-host.mdx)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9aa51f391
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| The Config Validation exposes the following API endpoints under the `/api/config-validation` prefix: | ||
|
|
||
| ### Snapshots | ||
|
|
||
| - `POST /config-validation/snapshot` - Create a new configuration snapshot |
There was a problem hiding this comment.
This section documents a full /api/config-validation API surface, but there are no corresponding server routes or handlers in the codebase (the registered API groups in internal/api/routes.go do not include any config-validation group). As written, users following these endpoint examples will always get 404 responses, so this should be clearly marked as not yet implemented or updated to match real endpoints.
Useful? React with 👍 / 👎.
|
|
||
| ```javascript | ||
| // Example using JavaScript WebSocket API | ||
| const ws = new WebSocket('ws://localhost:8080/terminal/crowdsec'); |
There was a problem hiding this comment.
The documented WebSocket URL omits the /api prefix, but terminal routes are mounted under the API group (/api/terminal/:container), and the frontend client also connects to /api/terminal/.... Using the URL in this example will fail with 404, so the sample should include /api to be runnable.
Useful? React with 👍 / 👎.
|
|
||
| ## Configuration | ||
|
|
||
| Hub behavior can be customized through environment variables or the configuration file. Refer to the [Configuration](../configuration/index.mdx) documentation for details. |
There was a problem hiding this comment.
This link points to ../configuration/index.mdx, but there is no docs/content/docs/configuration/index.mdx file in the repository, so readers are sent to a dead page. The new feature docs repeat this target in multiple places, so these references should be updated to an existing configuration doc (or an index page should be added).
Useful? React with 👍 / 👎.
2 participants
Documentation Changes to address:
Maintainer Alignment: Documentation preferences and open questions
Summary of changes made to CrowdSec Manager documentation:
Fixed Inaccuracies
Architecture Documentation:
Changed router from chi to gin-gonic/gin to match actual code
Updated Go version from 1.21+ to 1.23+ to match go.mod
API Response Format:
Corrected from {"status": "success", "data": {...}} to {"success": true, "data": {...}} to match actual Response struct
Added Documentation for Previously Undocumented Features
Hub Browser - Browse, install, upgrade CrowdSec collections, parsers, scenarios
Config Validation - Detect drift between running config and saved snapshots
History - Decision history with trends, charts, geographic data
Simulation Mode - Toggle CrowdSec simulation mode from UI
Terminal - Web-based shell into the CrowdSec container
Multi-Host Docker - Manage multiple Docker hosts via DOCKER_HOSTS env var
Each new feature document includes:
Feature description and capabilities
API endpoints with examples
Usage instructions
Configuration notes
All changes were made to files following the existing documentation structure and style.