Conversation
WalkthroughThis PR introduces HedgeDoc support to the configuration system by adding three new menu entries (HDOC01, HDOC02, HDOC03) in the JSON configuration file for install, remove, and purge operations under WebHosting, along with a corresponding shell module that implements the complete lifecycle management for HedgeDoc deployments via Docker with PostgreSQL backend integration. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tools/modules/software/module_hedgedoc.sh`:
- Line 46: Line " # Install postgres if not installed" uses spaces
for indentation; replace the leading spaces with a single tab character so the
comment aligns with the surrounding tab-indented blocks (match the indentation
style used by nearby lines in this file), e.g., update the line containing "#
Install postgres if not installed" to start with a tab instead of spaces.
- Around line 37-39: The snippet uses exit 0 inside the module_hedgedoc check
which will terminate the entire shell instead of just ending the function;
change the behavior to return 0 from the function when HedgeDoc is already
running. Locate the conditional that reads "if module_hedgedoc status; then exit
0 fi" and replace the exit call with a return 0 so only the current function
returns successfully and the caller script continues.
🧹 Nitpick comments (4)
tools/modules/software/module_hedgedoc.sh (4)
18-19: Unused variablecondition.The variable
conditionis assigned but never referenced anywhere in the function. Consider removing it to avoid confusion.🔧 Suggested fix
function module_hedgedoc () { local title="hedgedoc" - local condition=$(which "$title" 2>/dev/null)
54-54: Declaresession_secretas local.The variable
session_secretis not declared withlocal, which means it will leak into the outer scope. Other similar variables likeDATABASE_PASSWORDare properly declared as local.🔧 Suggested fix
- session_secret=$(openssl rand -hex 32) + local session_secret=$(openssl rand -hex 32)
70-80: Readiness check only verifies container is running, not application health.The loop checks
State.Runningbut HedgeDoc might still be initializing (database migrations, etc.). Consider checking for the health status or verifying the HTTP endpoint is responsive for more reliable startup detection.
69-69: Pin HedgeDoc image to a specific version instead of using:latest.Using
:latestcan lead to unexpected behavior when the upstream image is updated. Pin to a specific stable version (currently 1.10.5) for reproducibility, similar to PostgreSQL on line 48 which usespostgres:16-alpine.
| if module_hedgedoc status; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Using exit 0 instead of return 0 will terminate the calling script.
When HedgeDoc is already running, exit 0 terminates the entire shell process rather than just returning from the function. This could cause unexpected behavior if the function is called as part of a larger script.
🐛 Proposed fix
# Exit if hedgedoc is already running
if module_hedgedoc status; then
- exit 0
+ return 0
fi📝 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.
| if module_hedgedoc status; then | |
| exit 0 | |
| fi | |
| # Exit if hedgedoc is already running | |
| if module_hedgedoc status; then | |
| return 0 | |
| fi |
🤖 Prompt for AI Agents
In `@tools/modules/software/module_hedgedoc.sh` around lines 37 - 39, The snippet
uses exit 0 inside the module_hedgedoc check which will terminate the entire
shell instead of just ending the function; change the behavior to return 0 from
the function when HedgeDoc is already running. Locate the conditional that reads
"if module_hedgedoc status; then exit 0 fi" and replace the exit call with a
return 0 so only the current function returns successfully and the caller script
continues.
| local DATABASE_NAME="hedgedoc" | ||
| local DATABASE_PASSWORD=$(openssl rand -hex 8) | ||
|
|
||
| # Install postgres if not installed |
There was a problem hiding this comment.
Fix indentation: spaces instead of tabs.
The pipeline failure indicates that line 46 uses spaces for indentation instead of tabs, which violates the project's editorconfig rules.
🔧 Suggested fix
Replace the leading spaces with tabs on line 46 to match the rest of the file.
🧰 Tools
🪛 GitHub Actions: Maintenance: Check coding style
[error] 46-46: editorconfig-checker: Wrong indentation type (spaces instead of tabs) at line 46.
🤖 Prompt for AI Agents
In `@tools/modules/software/module_hedgedoc.sh` at line 46, Line " #
Install postgres if not installed" uses spaces for indentation; replace the
leading spaces with a single tab character so the comment aligns with the
surrounding tab-indented blocks (match the indentation style used by nearby
lines in this file), e.g., update the line containing "# Install postgres if not
installed" to start with a tab instead of spaces.
Description
Add support for Hedgedoc using PostgreSQL.
Issue reference:
Related documentation:
Implementation Details
Provide a detailed description of the implementation. Include the following:
Documentation Summary
Metadata Included:
Did you include the metadata (associative arrays) in the code? Ensure that metadata for modules, jobs, and runtime has been updated appropriately.
Document Generated:
Did you generate the updated documentation using
armbian-configng --doc? Confirm if the command was run to updateREADME.mdand provide any relevant details.Testing Procedure
Describe the tests you ran to verify your changes. Provide relevant details about your test configuration.
Checklist