Fixes #1069 Added "Testing Your Changes" Section in CONTRIBUTING.md#1070
Fixes #1069 Added "Testing Your Changes" Section in CONTRIBUTING.md#1070Shobhini wants to merge 2 commits intoopensource-society:masterfrom
Conversation
…lines and reordering of contribution steps
📝 WalkthroughWalkthroughCONTRIBUTING.md and README.md were updated to add explicit development environment setup and testing instructions, clarify backend Python venv usage and commands, update prerequisites and verification steps, and add troubleshooting guidance. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![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 (1)
CONTRIBUTING.md (1)
84-88: Consider adding fallback setup commands before backend run.At Line 84–88, startup assumes an existing virtual environment and installed dependencies. Adding a short fallback (
python3 -m venv venv,pip install -r requirements.txt) would make this more robust for first-time contributors.Suggested doc tweak
**Make sure the backend is running:** ```bash cd backup_existing_project/backend +python3 -m venv venv # if not already created source venv/bin/activate # or venv\Scripts\activate on Windows +pip install -r requirements.txt # if dependencies are not installed yet python3 run.py</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@CONTRIBUTING.mdaround lines 84 - 88, The startup instructions assume an
existing virtualenv and installed deps; update the snippet around the venv
activation and run.py steps to include fallback setup commands: if the virtual
environment doesn't exist, create it with python3 -m venv venv, then activate
(source venv/bin/activate or venv\Scripts\activate on Windows) and install
dependencies with pip install -r requirements.txt before running python3 run.py
so first-time contributors can set up and run run.py successfully.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@CONTRIBUTING.md:
- Line 24: Update the broken anchor in the CONTRIBUTING.md link by replacing the
incorrect targetREADME.md#-getting-started-developmentwith the correct
anchorREADME.md#getting-started-developmentso the link points to the## 🛠️ Getting Started (Development)heading in README.md.
Nitpick comments:
In@CONTRIBUTING.md:
- Around line 84-88: The startup instructions assume an existing virtualenv and
installed deps; update the snippet around the venv activation and run.py steps
to include fallback setup commands: if the virtual environment doesn't exist,
create it with python3 -m venv venv, then activate (source venv/bin/activate or
venv\Scripts\activate on Windows) and install dependencies with pip install -r
requirements.txt before running python3 run.py so first-time contributors can
set up and run run.py successfully.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `8a4674b5-7abe-4135-8b5b-05d00bc12e47` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5ea6d3456e598425e1b8e845912dc324929996ca and e5410b6192a74fe6fb8d7863695792658caa19b0. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `CONTRIBUTING.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…ification and troubleshooting tips
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
116-117: Split Bash and Windows activation instructions into separate snippets.Line 116 mixes a Windows command inside a bash block, which is easy to copy incorrectly. Prefer separate
bashandpowershell/cmdblocks for safer copy-paste usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 116 - 117, The README currently places a Windows activation command inside a bash code fence; split this into two separate code snippets: one bash block containing only the Unix activation command (source venv/bin/activate) and a separate Windows block (PowerShell/CMD) showing the appropriate Windows activation commands (e.g., venv\Scripts\Activate.ps1 for PowerShell and venv\Scripts\activate.bat for cmd). Replace the single mixed line "source venv/bin/activate # On Windows: venv\Scripts\activate" with these two distinct fenced blocks so copy/paste behavior is correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 86-90: The fenced code blocks in README containing the commands
"node --version", "npm --version", "python3 --version" (and the other two blocks
flagged) don’t match the repo’s configured fenced-code style and trigger MD046;
update those fenced blocks to the repository’s canonical style (use the
preferred fence marker and language tag, e.g. ```sh or the configured token),
ensure a blank line before and after each fence if required by the linter, and
make the same consistent change for the other flagged blocks so all README code
fences (including the ones showing node/npm/python commands) conform to the repo
rule.
- Around line 145-146: The README shows using `python3 run.py --port 5001` but
run.py currently calls app.run(debug=True) and does not parse CLI args, so the
flag is ignored; either update the README to remove the `--port` example and
document the supported method to change the port (e.g., set an environment
variable or edit the port in the file) or modify run.py to accept a --port
argument and pass it into app.run(port=...) (look for the main entry where
app.run(debug=True) is called and the module name run.py to implement argument
parsing or adjust the docs accordingly).
---
Nitpick comments:
In `@README.md`:
- Around line 116-117: The README currently places a Windows activation command
inside a bash code fence; split this into two separate code snippets: one bash
block containing only the Unix activation command (source venv/bin/activate) and
a separate Windows block (PowerShell/CMD) showing the appropriate Windows
activation commands (e.g., venv\Scripts\Activate.ps1 for PowerShell and
venv\Scripts\activate.bat for cmd). Replace the single mixed line "source
venv/bin/activate # On Windows: venv\Scripts\activate" with these two distinct
fenced blocks so copy/paste behavior is correct.
| ```bash | ||
| node --version | ||
| npm --version | ||
| python3 --version | ||
| ``` |
There was a problem hiding this comment.
Resolve MD046 warnings on newly added code blocks.
These fenced blocks are currently flagged by markdownlint (MD046) at Line 86, Line 113, Line 120, and Line 137. Please align these blocks with the repo’s configured code-block style so docs lint stays clean.
Also applies to: 113-113, 120-120, 137-137
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 86-86: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 86 - 90, The fenced code blocks in README containing
the commands "node --version", "npm --version", "python3 --version" (and the
other two blocks flagged) don’t match the repo’s configured fenced-code style
and trigger MD046; update those fenced blocks to the repository’s canonical
style (use the preferred fence marker and language tag, e.g. ```sh or the
configured token), ensure a blank line before and after each fence if required
by the linter, and make the same consistent change for the other flagged blocks
so all README code fences (including the ones showing node/npm/python commands)
conform to the repo rule.
| python3 run.py --port 5001 | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether backend run.py accepts/handles a --port argument.
# Expected:
# - If argparse/click/sys.argv handling exists for port, command is likely valid.
# - Otherwise, docs should be updated to the supported way (e.g., env var or app.run(port=...)).
set -euo pipefail
RUNPY_PATH="$(fd -t f 'run.py' | rg 'backup_existing_project/backend/run.py' || true)"
if [ -z "$RUNPY_PATH" ]; then
echo "run.py not found at expected location"
exit 0
fi
echo "Inspecting: $RUNPY_PATH"
nl -ba "$RUNPY_PATH" | sed -n '1,260p'
echo
echo "Searching for explicit CLI parsing related to port..."
rg -n -C2 'argparse|click|sys\.argv|--port|port\s*=|app\.run\(' "$RUNPY_PATH"Repository: opensource-society/notesvault
Length of output: 184
🏁 Script executed:
# Find and read the run.py file
cat backup_existing_project/backend/run.pyRepository: opensource-society/notesvault
Length of output: 173
🏁 Script executed:
# Also check the README.md context around lines 145-146
sed -n '140,150p' README.mdRepository: opensource-society/notesvault
Length of output: 310
Remove or correct the --port example on lines 145–146 since run.py does not parse CLI arguments.
The run.py file calls app.run(debug=True) with no argument parsing. The command python3 run.py --port 5001 will not work as documented—the --port flag will be ignored. Either update the documentation to show the supported way to change the port (e.g., environment variable if the app supports it, or modify the code directly), or add argument parsing to run.py to actually handle --port.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 145 - 146, The README shows using `python3 run.py
--port 5001` but run.py currently calls app.run(debug=True) and does not parse
CLI args, so the flag is ignored; either update the README to remove the
`--port` example and document the supported method to change the port (e.g., set
an environment variable or edit the port in the file) or modify run.py to accept
a --port argument and pass it into app.run(port=...) (look for the main entry
where app.run(debug=True) is called and the module name run.py to implement
argument parsing or adjust the docs accordingly).
1 participant
Provides step-by-step testing guidance for both frontend and backend contributions:
Frontend Testing
Backend Testing
Pre-Commit Checklist
Why This Matters
Summary by CodeRabbit