CONTRIBUTING.md still said "main is the product every site runs and the publication mirror exports" and framed wj/wilm as site branches feeding it. That was true when main was the only thing anyone deployed. It is not now: WJ production runs wj, the /ops instance runs dev, and main deploys nowhere. Nothing in the repository mentioned dev at all, so the branch a deploy script refuses to run without was undocumented. Replaces that section with the branch/instance table and the dev -> wj -> main promotion order, and says why promoting before validating is the wrong way round. Keeps the wilm note but moves it up with the reason it cannot be merged locally: it sits on the publication lineage and shares no ancestor. CLAUDE.md gains the branch-to-server pointer and records that export-github.sh now builds a dist only for wj and dev. docs/DEPLOY.md is deliberately left alone: `git pull origin main` is correct there. That document is the generic connected-Docker deploy another site follows, and main is exactly what another site adopts. Scope: product
248 lines
11 KiB
Markdown
248 lines
11 KiB
Markdown
# Contributing to ShopDB Flask
|
|
|
|
## Coding Standards
|
|
|
|
### Naming principles
|
|
|
|
1. Spell it out. If a non-technical user looking at a report would not understand a column at a glance, rename it.
|
|
2. No project-specific shorthand. A name either uses a word in full, or uses an acronym from the allowed list below.
|
|
3. Consistency beats cleverness. Once a name is committed and used in code, do not rename it for marginal improvement.
|
|
4. When unsure, ask: "would a new shop-floor IT person understand this?"
|
|
|
|
### Database
|
|
|
|
- **Table names**: lowercase, concatenated, plural. No dashes, no underscores.
|
|
- Good: `machines`, `pctypes`, `networkdevices`, `businessunits`, `auditlogs`
|
|
- Bad: `machine_types`, `network-devices`, `BusinessUnits`
|
|
- **Column names**: lowercase, concatenated, singular. No dashes, no underscores.
|
|
- Good: `machineid`, `machinenumber`, `lastzabbixsync`, `isactive`
|
|
- Bad: `machine_id`, `last_zabbix_sync`, `IsActive`
|
|
- **Foreign keys**: referenced table name (singular) + `id`.
|
|
- Good: `locationid` references `locations`, `vendorid` references `vendors`
|
|
- **Booleans**: prefix with `is` or `has`.
|
|
- Good: `isactive`, `isshopfloor`, `hasprinter`
|
|
- **Index names**: `idx_<table>_<column>`. Underscores allowed here only because indexes are infrastructure, not data.
|
|
- Good: `idx_machines_locationid`
|
|
|
|
### Migrations and deploy ordering
|
|
|
|
- **A model change ships WITH its migration in the same change.** Adding or
|
|
changing a column/table on a SQLAlchemy model without the matching Alembic
|
|
migration is a broken change, even if it passes tests against a freshly built
|
|
schema.
|
|
- **Code and schema must never deploy out of step.** A model that references a
|
|
column its migration has not applied yet makes MySQL raise
|
|
`1054 Unknown column`, which 500s every query on that table (this is not a
|
|
degraded read - the whole endpoint fails).
|
|
- **Bringing up a new image MUST run migrations before serving traffic**, in
|
|
order: `flask db upgrade` (core chain) then `flask plugin upgrade-all`
|
|
(per-plugin chains). A rebuilt image with new model code but an
|
|
un-migrated database will 500 until this runs.
|
|
- Core schema lives in `migrations/versions/`; each plugin owns its own chain
|
|
under `plugins/<name>/migrations/`. New schema = a new migration with an
|
|
idempotent guard and a real downgrade, clean on strict MySQL 8.
|
|
|
|
### Allowed acronyms (closed list, expand by PR)
|
|
|
|
- **Universal**: `id`, `url`, `api`, `http`, `https`, `json`, `jwt`, `sql`, `os`, `ip`, `dns`, `csv`, `pdf`, `cors`, `ttl`, `uuid`, `html`, `css`, `orm`
|
|
- **Domain (shop floor / IT infra)**: `cmm`, `cnc`, `pc`, `usb`, `vnc`, `winrm`, `ssh`, `ssl`, `tls`, `tcp`, `udp`, `smtp`, `ldap`, `vlan`, `sso`, `dnc`, `focas`, `clm`, `mtconnect`
|
|
|
|
Any acronym not on these lists must be spelled out. Proposing a new acronym is a separate PR that updates this list.
|
|
|
|
**Third-party imports are exempt.** Library and stdlib module paths cannot be renamed. `from sqlalchemy.orm import ...`, `import importlib.util`, `from werkzeug.utils import ...` are all fine even if they contain words that would otherwise be banned.
|
|
|
|
### Banned shorthand
|
|
|
|
`cfg`, `ctx`, `mgr`, `req`, `res`, `env`, `util`, `helper`, and any other domain-specific shortening. Spell it out: `config`, `context`, `manager`, `request`, `response`, `environment`, `utilities`. Note: `db` as a standalone variable name is banned, but `db` as a prefix in `database` or as part of project name `shopdb` is fine.
|
|
|
|
**Suffix exception.** Banned shorthand applies to standalone identifiers. As a suffix where the prefix establishes meaning, it is acceptable:
|
|
|
|
- `printers_bp`, `auth_bp`, `network_bp` are fine. The prefix names what blueprint, the `_bp` suffix is conventional for Flask blueprints.
|
|
- `request_obj` is fine when disambiguating from imported `request`.
|
|
- `bp` alone as a variable name is not fine. Always pair with a meaningful prefix.
|
|
|
|
### Code
|
|
|
|
#### Python
|
|
|
|
- **Variables and functions holding a database value**: match the column name exactly. No conversion to snake_case.
|
|
- DB column `machineid` -> Python variable `machineid`, attribute `Machine.machineid`, dict key `{"machineid": 1}`
|
|
- DB column `lastzabbixsync` -> Python variable `lastzabbixsync`
|
|
- **Pure code variables and functions** (loop counters, helpers, anything that does not mirror a DB field): snake_case per PEP 8.
|
|
- Good: `loop_count`, `current_user`, `validate_input()`
|
|
- **Classes**: PascalCase. Spell out fully.
|
|
- Good: `PrinterData`, `AssetType`, `NetworkDevice`
|
|
- **Modules / file names**: lowercase. Underscores allowed where it improves readability (PEP 8 permits this for modules).
|
|
- Good: `employee_service.py`, `asset_routes.py`
|
|
|
|
#### JavaScript / Vue
|
|
|
|
- **Variables**: camelCase per JS convention.
|
|
- Good: `currentUser`, `isLoading`
|
|
- **Variables holding API field values**: match the API field name exactly. Do NOT convert to camelCase.
|
|
- API returns `{"machineid": 1}` -> JS `response.machineid`, NOT `response.machineId`
|
|
- API returns `{"lastzabbixsync": "..."}` -> JS `device.lastzabbixsync`
|
|
- **Components**: PascalCase. Spell out fully.
|
|
- Good: `AssetDetail.vue`, `MachineForm.vue`, `PrinterList.vue`
|
|
- **CSS classes**: lowercase with dashes.
|
|
- Good: `asset-detail`, `machine-form`
|
|
|
|
#### API
|
|
|
|
- **Endpoints**: lowercase, plural nouns. No underscores or dashes.
|
|
- Good: `/api/machines`, `/api/networkdevices`, `/api/businessunits`
|
|
- **Query parameters**: lowercase, concatenated. Match column names where applicable.
|
|
- Good: `?locationid=5`, `?isactive=true`
|
|
- **Response keys**: match database column names exactly.
|
|
- Good: `{"machineid": 1, "lastzabbixsync": "2026-01-12T10:00:00Z"}`
|
|
|
|
### Examples
|
|
|
|
```python
|
|
# Good - DB-mirrored fields keep column names; pure code uses snake_case
|
|
class PrinterData(db.Model):
|
|
__tablename__ = 'printerdata'
|
|
|
|
machineid = db.Column(db.Integer, db.ForeignKey('machines.machineid'))
|
|
lastzabbixsync = db.Column(db.DateTime)
|
|
isnetworkprinter = db.Column(db.Boolean)
|
|
|
|
|
|
def list_printers_by_location(locationid: int) -> list[dict]:
|
|
printers = PrinterData.query.filter_by(locationid=locationid).all()
|
|
result_count = len(printers)
|
|
return [{"machineid": p.machineid, "isnetworkprinter": p.isnetworkprinter} for p in printers]
|
|
```
|
|
|
|
```python
|
|
# Bad - PEP 8-style underscores on DB fields, banned shorthand, project acronym not on allowed list
|
|
class PrinterData(db.Model):
|
|
__tablename__ = 'printer_data'
|
|
|
|
machine_id = db.Column(db.Integer)
|
|
last_zabbix_sync = db.Column(db.DateTime)
|
|
is_net_prn = db.Column(db.Boolean)
|
|
|
|
|
|
def list_prns_by_loc(loc_id):
|
|
bp_ctx = get_context()
|
|
cfg = load_cfg()
|
|
return PrinterData.query.filter_by(loc_id=loc_id).all()
|
|
```
|
|
|
|
### Style policy (non-naming)
|
|
|
|
- No emojis in code, comments, documentation, string literals, or UI.
|
|
- No em-dashes (U+2014), en-dashes (U+2013), Unicode arrows, or smart quotes anywhere. Plain ASCII only.
|
|
- Comments default to none. Add one only when the WHY is non-obvious. Keep comments terse (lite caveman style is fine for inline `#` and `//` comments). Docstrings stay normal English.
|
|
- Dark theme is default. Keep UI functional and professional.
|
|
|
|
### File structure
|
|
|
|
- Core models: `shopdb/core/models/`
|
|
- Core API routes: `shopdb/core/api/`
|
|
- Core services: `shopdb/core/services/`
|
|
- Plugin code: `plugins/<plugin>/{models,api,services,schemas}/`
|
|
|
|
### Plugin development
|
|
|
|
When creating a new plugin:
|
|
|
|
1. Create directory `plugins/<name>/`. Use the scaffold (`flask plugin new <name>`) when available.
|
|
2. Include `manifest.json` with metadata. This is the single source of truth for plugin name, version, dependencies, api_prefix.
|
|
3. Extend `BasePlugin` class.
|
|
4. Follow naming conventions above.
|
|
5. Run `flask plugin install <name>` to install.
|
|
|
|
See the PLUGIN-QUICKSTART wiki page (when published) for the 30-minute walkthrough.
|
|
|
|
## Testing
|
|
|
|
Run tests with:
|
|
|
|
```bash
|
|
pytest tests/
|
|
```
|
|
|
|
Tests are required for:
|
|
- New routes (smoke + happy path + 1-2 edge cases)
|
|
- New plugin hooks (contract test)
|
|
- Schema migrations (before/after data integrity)
|
|
|
|
## Branches: what belongs on main, and what does not
|
|
|
|
`main` is the curated product other sites adopt, and what the publication
|
|
mirror exports. It is a release decision: something reaching `main` says other
|
|
sites should take it. **`main` is not what this site runs.**
|
|
|
|
Each branch answers to one instance:
|
|
|
|
| branch | runs on | contains |
|
|
|---|---|---|
|
|
| `dev` | the dev instance | work being validated |
|
|
| `wj` | this site's PRODUCTION | everything this site runs |
|
|
| `main` | nothing directly | the curated release for other sites |
|
|
| `wilm` | the other site | their work, on the corporate GitHub |
|
|
|
|
So work flows `dev` -> `wj` -> `main`: land it on `dev`, validate it on the dev
|
|
instance, take it to `wj` for production, and promote to `main` what you are
|
|
happy for another site to run. Promoting first and back-filling the site branch
|
|
is the wrong way round - it puts unvalidated work in front of other sites and
|
|
leaves production behind.
|
|
|
|
`wilm` is not visible from the dev box because that machine has no account on
|
|
the corporate GitHub, which is why `fetch-wilm-branch.cmd` exists to bring it
|
|
back for review. It sits on the publication lineage, so it shares no ancestor
|
|
with `main`/`wj`/`dev` and cannot be merged locally.
|
|
|
|
A site branch is not a fork. Both feed `main`, and the sooner they do it the
|
|
cheaper it is.
|
|
|
|
**Most work is product.** ADR-015 gives site differences three homes on `main`:
|
|
a setting with a neutral default, seed data, or a site-namespaced directory
|
|
(`scripts/site_imports/<site>/`, which the style gate exempts). That is the only
|
|
site-specific home in the tree, so a site branch is about TIMING, not locality:
|
|
things that are not ready yet, rather than things that are permanently ours.
|
|
|
|
Say which at commit time, with a trailer, because the decision does not survive
|
|
being deferred - months later it is a list of subject lines and a guess:
|
|
|
|
```bash
|
|
git commit -m "Add the thing" -m "Scope: product" # main should have it
|
|
git commit -m "Add the thing" -m "Scope: site" # this site alone
|
|
```
|
|
|
|
Then:
|
|
|
|
```bash
|
|
tools/wj-status.sh # what is unpromoted, and where it belongs
|
|
tools/promote-to-main.sh --dry-run <sha> # what files would reach main
|
|
tools/promote-to-main.sh <sha> [<sha> ...] # oldest first; runs main's gates
|
|
tools/git-push.sh main # you push, deliberately
|
|
```
|
|
|
|
Windows equivalents are `tools\git-push.cmd` and `tools\promote-to-main.cmd`.
|
|
|
|
Promotion cherry-picks NAMED commits rather than merging the branch. Merging
|
|
takes the site work along with the product work, which is how a site branch
|
|
quietly becomes the product.
|
|
|
|
**The failure to watch for is not what you think.** Promoting something
|
|
site-specific by accident is caught by the ADR-015 gate on the way into `main`.
|
|
The real risk is product work sitting on a site branch until bringing it back is
|
|
a port rather than a merge - one such round trip cost a MySQL dialect fix, a
|
|
contract violation and a plugin install.
|
|
|
|
So rebase the site branch on `main` often, and promote as you go rather than in
|
|
one batch at the end.
|
|
|
|
## Code review checklist
|
|
|
|
- [ ] No underscores in table or column names
|
|
- [ ] No banned shorthand or unapproved acronyms in any new identifier
|
|
- [ ] DB-mirrored Python and JS variables match column names exactly
|
|
- [ ] API response keys match column names exactly
|
|
- [ ] No em-dashes, en-dashes, smart quotes, Unicode arrows, or emojis (run `grep -rP '[^\x00-\x7F]' --include='*.py' --include='*.vue' --include='*.js' .`)
|
|
- [ ] Plugin follows `BasePlugin` interface and contract version
|
|
- [ ] Tests included for new functionality
|