GRM-56: fix: close CI gaps with workflow dry-run, automated configure_repo, aligned timeouts
This commit is contained in:
@@ -38,6 +38,7 @@ Workflow YAML files (`.gitea/workflows/*.yml`) are verified with two tools:
|
||||
Both run via `make workflow-check` and are part of `make lint-all`.
|
||||
The pre-commit hook runs actionlint automatically when workflow files change.
|
||||
The CI `quality` job runs `make setup` (which installs all tools) then `make lint-all`.
|
||||
CI also runs a best-effort `make workflow-dryrun` step (skipped if act_runner is not installed in the CI Docker image).
|
||||
|
||||
## Architecture
|
||||
|
||||
@@ -52,7 +53,11 @@ Every change to master goes through this workflow. No exceptions.
|
||||
|
||||
### Branch Protection (Required Gitea Settings)
|
||||
|
||||
Configure the following branch protection rules for `master` in Gitea repo settings:
|
||||
Branch protection and labels are automatically configured by
|
||||
`scripts/configure_repo.py`, which runs as a `configure-repo` job in
|
||||
the post-merge workflow on every push to master.
|
||||
|
||||
The following rules are enforced for `master`:
|
||||
- **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
|
||||
@@ -91,9 +96,10 @@ docs: update README
|
||||
### 6. Review the PR (Mandatory — Before Adding ready-to-merge Label)
|
||||
|
||||
**Review checklist:** Every PR is reviewed against
|
||||
[REVIEW_CHECKLIST.md](REVIEW_CHECKLIST.md) — 10 categories covering
|
||||
[REVIEW_CHECKLIST.md](REVIEW_CHECKLIST.md) — 13 categories covering
|
||||
architecture, code quality, security, i18n, testing, performance,
|
||||
UX, documentation, workflow compliance, and maintainability.
|
||||
UX, documentation, workflow compliance, maintainability, resource
|
||||
management, backwards compatibility, and logging.
|
||||
|
||||
**Automated review (CI `pr-review` job):** Every PR triggers an automated
|
||||
review via `scripts/ci/pr_review.py`. This job posts a review with
|
||||
@@ -104,8 +110,11 @@ the **[auto]** items in the checklist:
|
||||
- Best practices (no `print()`, no bare `except`, no `TODO`/`FIXME`,
|
||||
no functions > 50 lines)
|
||||
- Security (no hardcoded secrets, no `shell=True`, no `eval`/`exec`)
|
||||
- i18n (no raw strings in `click.echo()` without `_()` wrapper)
|
||||
- Resource management (no `open()` without `with`, no `Popen()` without cleanup)
|
||||
- Documentation (source changes must include doc updates)
|
||||
- Test coverage (source changes must include test updates)
|
||||
- Commit conventions (conventional commit format on PR commits)
|
||||
|
||||
The automated review posts inline comments on specific lines and
|
||||
includes a link to the full checklist. The agent **must** address all
|
||||
@@ -129,17 +138,21 @@ Fix each comment one by one, commit, and push. Re-review until satisfied.
|
||||
|
||||
### 8. Approve and Merge
|
||||
Once all checklist items are verified and comments are addressed, post
|
||||
an approval review with `--checklist-confirmed`:
|
||||
an approval review with `--checklist-confirmed` and `--checklist-categories`:
|
||||
```bash
|
||||
REPO_TOKEN=<token> python3 scripts/ci/review_pr.py <pr_number> <owner/repo> \
|
||||
--event APPROVE --checklist-confirmed \
|
||||
--body "All 10 REVIEW_CHECKLIST.md categories verified. Architecture: <summary>. Security: <summary>. Tests: <summary>. Docs: <summary>."
|
||||
--checklist-categories 1,2,3,4,5,6,7,8,9,10,11,12,13 \
|
||||
--body "All 13 REVIEW_CHECKLIST.md categories verified. Architecture: <summary>. Security: <summary>. Tests: <summary>. Docs: <summary>."
|
||||
```
|
||||
|
||||
The `--checklist-confirmed` flag is **required** for APPROVE events —
|
||||
it attests that the reviewer has gone through every checklist category.
|
||||
The review body must also be substantive (> 20 characters) — trivial
|
||||
"LGTM" approvals are rejected by the auto-merge gate.
|
||||
The `--checklist-categories` flag is also **required** — it must list at
|
||||
least 8 of the 13 category numbers, ensuring the reviewer actually
|
||||
checked each category rather than rubber-stamping. The review body must
|
||||
be substantive (> 50 characters) — trivial approvals like "LGTM" are
|
||||
rejected.
|
||||
|
||||
Then add the `ready-to-merge` label. The auto-merge workflow will:
|
||||
1. **Validate** PR title format (`GRM-N: <vikunja task title>`) and match against Vikunja task title
|
||||
@@ -433,8 +446,8 @@ docs/
|
||||
### Documentation Coverage
|
||||
|
||||
- `scripts/ci/doc_coverage.py` checks that all CLI commands, Python modules, and CI scripts are documented
|
||||
- Runs as a CI step in the quality job
|
||||
- Goal: 100% coverage for public CLI commands and major architectural components
|
||||
- Runs as a CI step in the quality job with `--fail-on-missing` (blocks CI if docs are missing)
|
||||
- Enforced: 100% coverage for public CLI commands and major architectural components
|
||||
|
||||
### Updating Documentation
|
||||
|
||||
|
||||
Reference in New Issue
Block a user