docs: improve setup instructions, add .env.sample and basic test#1051
docs: improve setup instructions, add .env.sample and basic test#1051AkramZaabi wants to merge 1 commit into
Conversation
👀 Peer Review RequiredHi @AkramZaabi! This pull request does not yet have a peer review. Before this PR can be merged, please request a review from one of your peers:
Thank you for contributing! 🎉 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR updates the development environment configuration and documentation baseline. Changes include restructuring Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ 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 |
🚚 This Repository Is MovingHi @AkramZaabi, thank you for your contribution! We are in the process of migrating most of the logic from this repository to our new repository: alphaonelabs/learn. What this means for your PRPlease do not merge or continue work here. Instead:
This PR has been automatically closed. Once you have opened your PR in the new repository, feel free to reference it here. Thank you for your understanding and continued support! 🙏 |
There was a problem hiding this comment.
Pull request overview
Improves contributor onboarding and configuration hygiene by updating setup documentation, refreshing .env.sample to match runtime configuration, and adding a regression test to keep environment variables aligned over time.
Changes:
- Rewrites
README.mdsetup flow with cross-platform steps and clearer ASGI/Uvicorn guidance. - Reworks
.env.sampleby removing stale flags and grouping runtime env vars with comments. - Adds a Django test to assert
.env.samplecovers env vars referenced by runtime settings.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
web/tests/test_env_sample.py |
Adds a regression test to ensure .env.sample stays aligned with runtime env var usage. |
README.md |
Updates local dev setup instructions, ASGI/Uvicorn startup, admin URL notes, and common errors. |
.github/copilot-setup-steps.yml |
Syncs Copilot setup steps with the updated recommended Uvicorn dev server flow. |
.env.sample |
Updates and reorganizes sample environment variables to match current settings/runtime expectations. |
| git clone https://github.com/alphaonelabs/alphaonelabs-education-website.git | ||
| cd alphaonelabs-education-website |
There was a problem hiding this comment.
The README clone URL (alphaonelabs/alphaonelabs-education-website.git) and the suggested cd directory name don’t match other repository references in this repo (e.g., pyproject.toml uses the project name education-website, and multiple docs link to github.com/alphaonelabs/education-website). This inconsistency is likely to break onboarding instructions; please align the clone/cd commands with the canonical repo name used elsewhere in the project.
| git clone https://github.com/alphaonelabs/alphaonelabs-education-website.git | |
| cd alphaonelabs-education-website | |
| git clone https://github.com/alphaonelabs/education-website.git | |
| cd education-website |
| python manage.py create_test_data | ||
| ``` | ||
| ```bash | ||
| python web/master_key.py |
There was a problem hiding this comment.
The command to generate a messaging key uses python web/master_key.py, but earlier sections note that commands may be run via poetry run. If a contributor follows the poetry run approach without activating the venv, this command may run against the wrong interpreter and fail/miss dependencies. Consider using poetry run python web/master_key.py (or explicitly state it must be run inside the activated venv).
| python web/master_key.py | |
| poetry run python web/master_key.py |
| name: "Run Development Server" | ||
| description: "Start the Django development server" | ||
| commands: | ||
| - "python manage.py runserver" | ||
| - "poetry run uvicorn web.asgi:application --host 127.0.0.1 --port 8000 --reload" | ||
| notes: | ||
| - "Server runs at http://localhost:8000" | ||
| - "Admin panel available at http://localhost:8000/admin" | ||
| - "Server runs at http://127.0.0.1:8000/en/" | ||
| - "Use Uvicorn for local WebSocket support" | ||
| - "Admin panel path is controlled by ADMIN_URL in .env" |
There was a problem hiding this comment.
This step’s description says “Start the Django development server”, but the command now starts Uvicorn (ASGI). Please update the name/description to reflect that this is an ASGI/Uvicorn startup step to avoid confusion when following the setup guide.
| SECRET_KEY=django-insecure-change-me | ||
|
|
||
| # Must be a valid Fernet key. Generate one with: python web/master_key.py | ||
| MESSAGE_ENCRYPTION_KEY=5ezrkqK2lhifqBRt9f8_dZhFQF_f5ipSQDV8Vzv9Dek= |
There was a problem hiding this comment.
.env.sample currently ships a concrete MESSAGE_ENCRYPTION_KEY value. Because this key protects encrypted messaging data, providing a real (valid) key in-repo increases the risk that someone deploys with the sample key unchanged. Safer pattern is to leave it blank (or set an obvious placeholder) and document generating a per-environment key (settings.py already has a default generator).
| MESSAGE_ENCRYPTION_KEY=5ezrkqK2lhifqBRt9f8_dZhFQF_f5ipSQDV8Vzv9Dek= | |
| MESSAGE_ENCRYPTION_KEY= |
| if line.strip() and not line.lstrip().startswith("#") and "=" in line | ||
| } | ||
|
|
||
| referenced_variables = set(re.findall(r'env\.(?:str|bool|list)\("([A-Z0-9_]+)"', settings_source)) |
There was a problem hiding this comment.
The env var extraction regex misses env("VAR", ...) usages (e.g., the STRIPE_* settings in web/settings.py use env(...) rather than env.str(...)), so this test can pass even when .env.sample is missing variables that are actually required at runtime. Consider expanding the regex to include env\("..." calls (and ideally also env.db()/other helpers) so the alignment check is meaningful.
| referenced_variables = set(re.findall(r'env\.(?:str|bool|list)\("([A-Z0-9_]+)"', settings_source)) | |
| referenced_variables = set( | |
| re.findall(r'env\.(?:str|bool|list|int|float|db)\("([A-Z0-9_]+)"', settings_source) | |
| ) | |
| referenced_variables.update(re.findall(r'env\("([A-Z0-9_]+)"', settings_source)) |
Summary
Why This Helps
Testing
python -m py_compile)Summary
This PR improves onboarding documentation and setup instructions for new contributors by streamlining the README, standardizing environment configuration, and adding automated validation tests.
Key Changes
README.md - Complete rewrite replacing outdated documentation with a simplified "Quick Start" workflow:
ADMIN_URLenvironment variable.env.sample - Reorganized from production-oriented to development-focused baseline:
DEBUG,DJANGO_DEBUGflags; deployment/VPS-specific fields).github/copilot-setup-steps.yml - Updated to align with new instructions:
poetry run uvicorninstead ofmanage.py runserver127.0.0.1:8000/en/with admin URL controlled via.envweb/tests/test_env_sample.py - New regression test suite:
.env.samplecontains all variables referenced in runtime settings.env.sampledoesn't contain deprecated debug flagsImpact