Improve contributor experience by separating backend and frontend run commands issue(fixes #837) #858
Conversation
|
|
📝 WalkthroughWalkthroughDocumentation enhancements added to YOLO utility functions with comprehensive docstrings for NMS, IoU computation, and visualization methods. Two new npm scripts introduced in package.json to independently run backend (uvicorn) and frontend (Tauri) components. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
|
|
1 similar 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 (1)
backend/app/utils/YOLO.py (1)
94-439: Consider separating unrelated changes into a separate PR.While the docstring additions are valuable and well-executed, they are unrelated to the PR's stated objective of improving the contributor experience with run commands. Mixing unrelated changes in a single PR can:
- Complicate code review and make it harder to track the purpose of changes
- Increase the risk surface if a rollback is needed
- Make it harder to trace changes through git history
For future PRs, consider keeping documentation improvements in a separate PR from feature/tooling changes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/utils/YOLO.pypackage.json
🔇 Additional comments (2)
backend/app/utils/YOLO.py (2)
94-107: Well-written docstrings improve code maintainability.The Google-style docstrings are comprehensive, clearly document parameters and return values, and significantly improve the developer experience when working with these YOLO utility functions.
Also applies to: 131-142, 174-183, 206-215, 229-243, 289-302, 316-332, 369-381
144-168: Helpful inline comments enhance code readability.The inline comments effectively explain the logic flow and intent, making the code easier to understand and maintain.
Also applies to: 245-278, 334-363, 383-395
| "frontend": "cd frontend && npm run tauri dev", | ||
| "backend": "cd backend && python -m uvicorn main:app --reload", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify backend dependencies and main.py structure
echo "=== Checking for uvicorn in backend dependencies ==="
fd -t f 'requirements.txt|pyproject.toml|Pipfile' backend/ --exec rg -n "uvicorn" {} \;
echo -e "\n=== Checking for main.py in backend directory ==="
fd -t f '^main\.py$' backend/
echo -e "\n=== Verifying app instance in main.py ==="
fd -t f '^main\.py$' backend/ --exec rg -n "app\s*=.*FastAPI|app\s*=.*Starlette" {} \;
echo -e "\n=== Checking backend directory structure ==="
ls -la backend/ 2>/dev/null || echo "backend/ directory not found"Repository: AOSSIE-Org/PictoPy
Length of output: 1147
Python command compatibility needs addressing.
The backend setup is properly configured: uvicorn is specified in requirements.txt (v0.30.1), backend/main.py exists with a FastAPI app instance. However, the npm script uses python which may fail on systems where only python3 is available. Consider using python3 instead or documenting the requirement. Note that you already have platform-specific scripts (run.sh and run-server.ps1), which suggests applying similar cross-platform considerations to the npm command would be beneficial.
🤖 Prompt for AI Agents
In package.json around lines 7-8, the backend npm script uses `python` which can
fail on systems where only `python3` is available; change the script to invoke
`python3 -m uvicorn main:app --reload` (or add a cross-platform fallback wrapper
that tries `python3` then `python`) and/or document the requirement in README so
the backend start command works reliably across platforms.
V4Vikaskumar
changed the title
|
|
1 participant
Summary
This PR adds two explicit scripts to the main
package.jsonto improve the local development experience:npm run backend– to run the backend servernpm run frontend– to run the frontend (Tauri app)Why this change was needed
New contributors often face multiple issues while setting up and running the project locally.
Previously, there was confusion around how to start the backend and frontend together, which made the initial setup harder and more error-prone.
By introducing separate commands:
Why a single command was not used
Running both the backend and frontend with a single command is not reliable in this project.
When the backend is started, it runs a long-living server process in the terminal.
Because of this, the same terminal cannot be used to start the frontend, which leads to blocking behavior and process management issues.
Due to this limitation, running both services from a single command caused repeated failures and instability.
Separating them into two commands provides a clearer, more stable, and predictable development workflow.
How to Run Backend and Frontend Locally
Two.Command.Problem.solve.mp4
Impact
This change does not alter application behavior and only improves the development setup.
issue (Fixes #837)