101 lines
4.7 KiB
Markdown
101 lines
4.7 KiB
Markdown
# Contributing Guide
|
|
|
|
## Key Conventions
|
|
|
|
- Python 3.12+ required (ruff/pyright target `py312`)
|
|
- 100% test coverage required (`--cov-fail-under=100`)
|
|
- Conventional commits on feature branches (no `GRM-N:` prefix)
|
|
- Branch names must include `GRM-N` task ID
|
|
- Line length: 120 chars
|
|
- Secrets are passed via temp JSON files, never on the command line (CWE-214)
|
|
- CI triggers only on `opened` and `synchronize` PR events (not `labeled`)
|
|
|
|
## Code Style Rules
|
|
|
|
- **Python version**: 3.12+ (ruff and pyright target `py312`)
|
|
- **Line length**: 120 characters
|
|
- **Test coverage**: 100% required (`--cov-fail-under=100`)
|
|
- **Secrets handling**: Secrets are passed via temp JSON files with `0600` permissions, never on the command line (CWE-214). Extra-vars are written to a temporary JSON file and passed via `--extra-vars @tempfile`, which is deleted after execution. This prevents secrets from being visible in the process list (`ps aux`).
|
|
- **Linting**: `make lint-all` runs ruff + pyright + bandit + ansible-lint + checkmake
|
|
|
|
## Commit Rules
|
|
|
|
Branch commits use conventional commit format (no `GRM-N:` prefix):
|
|
|
|
```
|
|
feat: add new feature
|
|
fix: resolve bug
|
|
docs: update README
|
|
```
|
|
|
|
### Version Bumping Rules
|
|
|
|
| Commit type | Version bump |
|
|
|-------------|-------------|
|
|
| `feat:` | minor (0.X.0) |
|
|
| `fix:` | patch (0.0.X) |
|
|
| `feat!:` or `BREAKING CHANGE` | minor (pre-1.0: major would be 1.0.0) |
|
|
| `chore:`, `ci:`, `docs:` | no bump (excluded by cliff.toml) |
|
|
|
|
## Branch Naming
|
|
|
|
| What | Format | Example |
|
|
|------|--------|---------|
|
|
| Branch name | `GRM-N-short-description` | `GRM-33-add-pr-review-step` |
|
|
| Branch commits | `<conventional commit>` | `feat: add review script` |
|
|
| PR title | `GRM-N: <vikunja task title>` | `GRM-33: Add mandatory PR review step` |
|
|
| Merge commit | `GRM-N <conventional commit>` | `GRM-33 feat: add review script` |
|
|
|
|
## PR Workflow Summary
|
|
|
|
Every change to master goes through this workflow. No exceptions.
|
|
|
|
1. **Create Vikunja task** — get a `GRM-N` identifier (Vikunja project 6)
|
|
2. **Create branch** — `GRM-N-short-description`
|
|
3. **Implement** — write code, tests (100% coverage), update docs
|
|
4. **Commit** — conventional commits (no `GRM-N:` prefix on branch)
|
|
5. **Push & create PR** — title: `GRM-N: <vikunja task title>`, body: summary + `Closes GRM-N`
|
|
6. **Review** — review the full diff focusing on: functional completeness, edge cases, technical excellence (architecture, SRP, deduplication, code smells, best practices, code quality, reusability, clean code, readability, maintainability, extensibility), performance, security, UX, documentation completeness/relevance. Post review comments via `devx.ci.review_pr`.
|
|
7. **Address comments** — fix each comment, commit, push, re-review
|
|
8. **Approve** — post an `APPROVE` review via `devx.ci.review_pr`
|
|
9. **Add `ready-to-merge` label** — auto-merge workflow squash-merges with title `GRM-N <conventional commit message>`, post-merge workflow marks the Vikunja task as done, release workflow automatically versions and tags
|
|
|
|
### Branch Protection (Required Gitea Settings)
|
|
|
|
Configure the following branch protection rules for `master` in Gitea repo settings:
|
|
|
|
- **Require pull request**: No direct pushes to master
|
|
- **Require approval review**: At least 1 `APPROVE` review before merge
|
|
- **Require status checks**: CI quality + molecule tests must pass
|
|
- **Block force pushes**: No history rewriting on master
|
|
|
|
The auto-merge workflow enforces the APPROVE review check programmatically as a defense-in-depth measure, but branch protection is the primary gate.
|
|
|
|
## Build & Test Commands
|
|
|
|
```bash
|
|
make setup # Create venv, install deps, set up hooks
|
|
make lint-all # ruff + pyright + bandit + ansible-lint + checkmake
|
|
make pytest-cov # Unit tests with 100% coverage enforcement
|
|
make test-unit # Unit tests without coverage
|
|
make molecule # All 6 scenarios on Ubuntu 22.04
|
|
make molecule-all # All 6 scenarios on all 4 supported OSes
|
|
make test-all # pytest-cov + molecule
|
|
```
|
|
|
|
## Ansible Role Conventions
|
|
|
|
```
|
|
main.yml → systemd_check → user_setup → rootless_docker → install_runner → prune → integration_test
|
|
```
|
|
|
|
- `install_runner.yml` handles: download, config, validate, register, service
|
|
- `main.yml` handles: prune, integration_test (NOT install_runner — avoids duplicates)
|
|
- `systemctl --user` tasks must be guarded by `docker_rootless_setup`
|
|
- Template creation tasks are NOT guarded by `docker_rootless_setup` (they just create files)
|
|
|
|
## Known Issues
|
|
|
|
- `ansible-lint` may warn about `command-instead-of-module` for `systemctl --user` calls — this is expected (systemd module doesn't support user services) and skipped in `.ansible-lint`
|
|
- Molecule Docker driver may print "Event loop is closed" warnings on interrupt — harmless
|