Tighten PR template and CONTRIBUTING to gate visual/style changes
The post-launch PR flood from LLM coding agents drowned the repo in PRs that don't run the app, attach no screenshots, and invent parallel component styling. Even tiny correctness fixes accumulated into a visual mess. Make the rules explicit in both the PR template and CONTRIBUTING: - Run the app and view the change in a browser before submitting. - Required screenshot for any UI/render touch (no longer "delete if not UI"). - Explicit style requirements: reuse CSS variables, no Unicode emoji (use SVG icons), monospaced font, dark theme, no parallel widgets. - Direct callout for bulk agent-generated PRs: open an issue first. PRs that ignore these will be closed without merge, regardless of code correctness.
This commit is contained in:
19
.github/pull_request_template.md
vendored
19
.github/pull_request_template.md
vendored
@@ -23,6 +23,7 @@ Fixes #
|
||||
- [ ] I searched [open issues](https://github.com/pewdiepie-archdaemon/odysseus/issues) and [open PRs](https://github.com/pewdiepie-archdaemon/odysseus/pulls) — this is not a duplicate.
|
||||
- [ ] This PR targets `main`
|
||||
- [ ] My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
|
||||
- [ ] I actually ran the app (`docker compose up` or `uvicorn app:app`) and verified the change works end-to-end. Type-checks and unit tests are not enough.
|
||||
|
||||
## How to Test
|
||||
|
||||
@@ -33,6 +34,20 @@ Fixes #
|
||||
2.
|
||||
3.
|
||||
|
||||
## Screenshots (UI changes only)
|
||||
## Visual / UI changes — REQUIRED if you touched anything that renders
|
||||
|
||||
<!-- Drag and drop images or a screen recording here. Delete this section if not a UI change. -->
|
||||
**Anything that changes what the UI looks like — buttons, icons, padding, colors, fonts, spacing, layout, CSS, HTML, SVG, or any `static/js/` module that draws to the DOM — needs all of the following. PRs that change rendering without these WILL be closed.**
|
||||
|
||||
- [ ] **Screenshot or short clip** of the change in the running app, attached below. Mobile screenshot too if the change affects mobile.
|
||||
- [ ] **Style match**: the change uses Odysseus's existing visual language. Specifically:
|
||||
- Reuse existing CSS variables (`--red`, `--fg`, `--bg`, `--card`, `--border`, etc.) — do not introduce new color values, font sizes, or spacing units.
|
||||
- Reuse existing button/input/card/border classes. Don't invent parallel styling.
|
||||
- **No Unicode emoji in UI or code.** Use inline SVG (matching the monochrome icon style already in `static/index.html`) or plain text.
|
||||
- Monospaced font (`Fira Code`) for primary UI text. Don't override.
|
||||
- Dark theme is the default; any light-mode work must be wired through the existing theme system, not hard-coded.
|
||||
- [ ] **No new component patterns.** If a similar widget already exists in the app, extend it instead of writing a parallel one.
|
||||
- [ ] **I am not an LLM agent submitting a bulk PR.** If you are, please open an issue describing the problem first — bulk auto-generated PRs that don't match the project's visual style are closed on sight, even when the underlying fix is correct.
|
||||
|
||||
### Screenshots / clips
|
||||
|
||||
<!-- Drag and drop images or a screen recording here. Required for any UI/visual change. -->
|
||||
|
||||
@@ -57,12 +57,32 @@ Good pull requests usually include:
|
||||
|
||||
- A short explanation of the bug or feature.
|
||||
- The files or areas changed.
|
||||
- Manual test steps or automated test results.
|
||||
- Manual test steps or automated test results from running the actual app, not just the test suite.
|
||||
- Screenshots or short recordings for UI changes.
|
||||
- Links to related issues, for example `Fixes #123`.
|
||||
|
||||
Please keep PRs small. Large PRs that mix unrelated cleanup, formatting, refactors, and behavior changes are much harder to review.
|
||||
|
||||
> **Auto-generated PRs.** If you are running an LLM agent (Devin, Cursor, OpenHands, Claude Code, etc.) against this repo: please open an issue describing the problem first instead of opening a PR directly. Bulk agent-generated PRs that don't match the project's visual style or contribution format will be closed without review, even when the underlying fix is correct.
|
||||
|
||||
## Style and visual changes
|
||||
|
||||
Odysseus has an intentional visual style. PRs that ignore it will be closed without merge, no matter how correct the underlying code is.
|
||||
|
||||
Before submitting any change that affects what the app looks like — buttons, icons, fonts, colors, spacing, layout, CSS, HTML, SVG, or any `static/js/` module that draws to the DOM — please:
|
||||
|
||||
1. **Run the app locally** and view the change in a browser. Type-checks and unit tests are not enough.
|
||||
2. **Attach a screenshot or short clip** of the change in the running app. Add a mobile screenshot too if the change affects mobile.
|
||||
3. **Match the existing visual language.** Specifically:
|
||||
- Reuse existing CSS variables (`--red`, `--fg`, `--bg`, `--card`, `--border`, …). Do not introduce new color values, font sizes, or spacing units.
|
||||
- Reuse existing button, input, card, and border classes. Don't invent parallel styling for similar widgets.
|
||||
- **No Unicode emoji in UI or code.** Use inline SVG (matching the monochrome icon style already in `static/index.html`) or plain text.
|
||||
- Monospaced font (`Fira Code`) for primary UI text. Don't override.
|
||||
- Dark theme is the default; any light-mode work goes through the existing theme system, not hard-coded.
|
||||
4. **Don't add parallel components.** If a similar widget already exists in the app, extend it instead of writing a new one.
|
||||
|
||||
If you are unsure whether a change is "visual," it is. Default to attaching a screenshot.
|
||||
|
||||
## Issue Reports
|
||||
|
||||
For bugs, include:
|
||||
|
||||
Reference in New Issue
Block a user