mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
ci: enforce PR issue references and branch conventions
Validate BO-<issue>-<slug> branch names and lint PR titles and commit headers in CI and release workflows. Require GitHub closing-keyword issue references and verify referenced issues exist. Align contributing, release, commit, testing, and policy docs with the enforced behavior and tone.
This commit is contained in:
+1
-1
@@ -4,4 +4,4 @@
|
||||
|
||||
Use GitHub's [private vulnerability reporting](https://github.com/bookorbit/bookorbit/security/advisories/new) to report security issues confidentially. Do not open public issues for security vulnerabilities.
|
||||
|
||||
I aim to acknowledge reports within a few days and publish a fix as quickly as the severity warrants.
|
||||
Reports are typically acknowledged within a few days, and fixes are published as quickly as severity warrants.
|
||||
|
||||
@@ -13,7 +13,13 @@
|
||||
|
||||
## What does this PR do?
|
||||
|
||||
<!-- A short summary of the change and why it's needed. Link the issue it addresses if applicable. -->
|
||||
<!--
|
||||
A short summary of the change and why it's needed.
|
||||
Link at least one issue using a GitHub closing keyword, for example:
|
||||
- Closes #123
|
||||
- Fixes: #123
|
||||
- RESOLVED owner/repo#123
|
||||
-->
|
||||
|
||||
Closes #
|
||||
|
||||
|
||||
+151
-3
@@ -38,12 +38,153 @@ jobs:
|
||||
- name: Dependency review
|
||||
uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0
|
||||
|
||||
pr-conventions:
|
||||
name: PR Conventions
|
||||
if: github.event_name == 'pull_request'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
contents: read
|
||||
issues: read
|
||||
pull-requests: read
|
||||
steps:
|
||||
- name: Validate PR branch name
|
||||
shell: bash
|
||||
env:
|
||||
PR_BRANCH: ${{ github.head_ref }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
pattern='^BO-[0-9]+-[a-z0-9]+(-[a-z0-9]+)*$'
|
||||
if [[ ! "$PR_BRANCH" =~ $pattern ]]; then
|
||||
echo "::error::Branch name '$PR_BRANCH' is invalid."
|
||||
echo "::error::Use BO-<issue-number>-<short-description> (example: BO-123-fix-reader-sync)."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Validate referenced issues exist
|
||||
shell: bash
|
||||
env:
|
||||
GH_REPO: ${{ github.repository }}
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
PR_BRANCH: ${{ github.head_ref }}
|
||||
PR_BODY: ${{ github.event.pull_request.body }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
validate_issue_exists() {
|
||||
local issue_repo="$1"
|
||||
local issue_number="$2"
|
||||
local reference_context="$3"
|
||||
local response_file
|
||||
local http_code
|
||||
local api_url
|
||||
local state
|
||||
local reference_label
|
||||
|
||||
response_file="$(mktemp)"
|
||||
api_url="https://api.github.com/repos/${issue_repo}/issues/${issue_number}"
|
||||
http_code="$(curl -sS -o "${response_file}" -w "%{http_code}" \
|
||||
-H "Authorization: Bearer ${GH_TOKEN}" \
|
||||
-H "Accept: application/vnd.github+json" \
|
||||
"${api_url}")"
|
||||
|
||||
if [[ "${issue_repo}" == "${GH_REPO}" ]]; then
|
||||
reference_label="#${issue_number}"
|
||||
else
|
||||
reference_label="${issue_repo}#${issue_number}"
|
||||
fi
|
||||
|
||||
if [[ "${http_code}" != "200" ]]; then
|
||||
echo "::error::${reference_context} references ${reference_label}, but it does not exist or is not accessible."
|
||||
rm -f "${response_file}"
|
||||
return 1
|
||||
fi
|
||||
|
||||
if jq -e '.pull_request != null' "${response_file}" > /dev/null; then
|
||||
echo "::error::${reference_context} references ${reference_label}, but that number is a pull request, not an issue."
|
||||
rm -f "${response_file}"
|
||||
return 1
|
||||
fi
|
||||
|
||||
state="$(jq -r '.state // "unknown"' "${response_file}")"
|
||||
echo "${reference_context}: issue ${reference_label} exists (state=${state})."
|
||||
rm -f "${response_file}"
|
||||
}
|
||||
|
||||
if [[ "${PR_BRANCH}" =~ ^BO-([0-9]+)- ]]; then
|
||||
branch_issue_number="${BASH_REMATCH[1]}"
|
||||
else
|
||||
echo "::error::Unable to extract issue number from branch '${PR_BRANCH}'."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
validate_issue_exists "${GH_REPO}" "${branch_issue_number}" "Branch name"
|
||||
|
||||
closing_keyword_pattern='(close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)'
|
||||
issue_reference_pattern="(^|[^[:alnum:]_])${closing_keyword_pattern}([[:space:]]*:[[:space:]]*|[[:space:]]+)([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#[1-9][0-9]*"
|
||||
issue_matches="$(printf '%s\n' "${PR_BODY}" | grep -Eio "${issue_reference_pattern}" || true)"
|
||||
if [[ -z "${issue_matches}" ]]; then
|
||||
echo "::error::PR description must include at least one GitHub closing-keyword issue reference (for example: 'Closes #123', 'Fixes: #123', or 'RESOLVED owner/repo#123')."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
issue_refs="$(printf '%s\n' "${issue_matches}" \
|
||||
| tr '[:upper:]' '[:lower:]' \
|
||||
| sed -E 's/^[^a-z0-9_]*//' \
|
||||
| sed -E "s/^${closing_keyword_pattern}([[:space:]]*:[[:space:]]*|[[:space:]]+)//" \
|
||||
| sort -u)"
|
||||
while IFS= read -r issue_ref; do
|
||||
[[ -z "${issue_ref}" ]] && continue
|
||||
|
||||
if [[ "${issue_ref}" =~ ^([a-z0-9_.-]+/[a-z0-9_.-]+)#([1-9][0-9]*)$ ]]; then
|
||||
issue_repo="${BASH_REMATCH[1]}"
|
||||
issue_number="${BASH_REMATCH[2]}"
|
||||
elif [[ "${issue_ref}" =~ ^#([1-9][0-9]*)$ ]]; then
|
||||
issue_repo="${GH_REPO}"
|
||||
issue_number="${BASH_REMATCH[1]}"
|
||||
else
|
||||
echo "::error::Unable to parse issue reference '${issue_ref}' in PR description."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
validate_issue_exists "${issue_repo}" "${issue_number}" "PR description"
|
||||
done <<< "${issue_refs}"
|
||||
|
||||
- name: Checkout
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
||||
with:
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Set up Node.js and pnpm
|
||||
uses: ./.github/actions/setup-node-pnpm
|
||||
with:
|
||||
node-version: ${{ env.NODE_VERSION }}
|
||||
pnpm-version: ${{ env.PNPM_VERSION }}
|
||||
|
||||
- name: Lint PR title
|
||||
shell: bash
|
||||
env:
|
||||
PR_TITLE: ${{ github.event.pull_request.title }}
|
||||
run: printf '%s\n' "$PR_TITLE" | pnpm exec commitlint --config commitlint.config.cjs
|
||||
|
||||
- name: Lint PR commit headers
|
||||
shell: bash
|
||||
env:
|
||||
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
pnpm exec commitlint --config commitlint.config.cjs --from "$PR_BASE_SHA" --to "$PR_HEAD_SHA" --verbose
|
||||
|
||||
detect-changes:
|
||||
name: Detect Changed Paths
|
||||
runs-on: ubuntu-latest
|
||||
needs: [dependency-review]
|
||||
needs: [dependency-review, pr-conventions]
|
||||
timeout-minutes: 10
|
||||
if: always() && (needs.dependency-review.result == 'success' || needs.dependency-review.result == 'skipped')
|
||||
if: >
|
||||
always() &&
|
||||
(needs.dependency-review.result == 'success' || needs.dependency-review.result == 'skipped') &&
|
||||
(needs.pr-conventions.result == 'success' || needs.pr-conventions.result == 'skipped')
|
||||
outputs:
|
||||
server: ${{ steps.manual.outputs.server || steps.changes.outputs.server }}
|
||||
client: ${{ steps.manual.outputs.client || steps.changes.outputs.client }}
|
||||
@@ -321,7 +462,13 @@ jobs:
|
||||
quality-gate:
|
||||
name: Quality Gate
|
||||
runs-on: ubuntu-latest
|
||||
needs: [dependency-review, detect-changes, lint, typecheck, tests]
|
||||
needs:
|
||||
- dependency-review
|
||||
- pr-conventions
|
||||
- detect-changes
|
||||
- lint
|
||||
- typecheck
|
||||
- tests
|
||||
timeout-minutes: 5
|
||||
if: always()
|
||||
steps:
|
||||
@@ -334,6 +481,7 @@ jobs:
|
||||
- name: Quality checks complete
|
||||
run: |
|
||||
echo "dependency-review=${{ needs.dependency-review.result }}"
|
||||
echo "pr-conventions=${{ needs.pr-conventions.result }}"
|
||||
echo "detect-changes=${{ needs.detect-changes.result }}"
|
||||
echo "lint=${{ needs.lint.result }}"
|
||||
echo "typecheck=${{ needs.typecheck.result }}"
|
||||
|
||||
@@ -20,16 +20,19 @@ env:
|
||||
|
||||
jobs:
|
||||
commitlint:
|
||||
name: Lint PR Title
|
||||
name: Lint PR Conventions
|
||||
if: github.event_name == 'pull_request'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
contents: read
|
||||
issues: read
|
||||
pull-requests: read
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
||||
with:
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Set up Node.js and pnpm
|
||||
uses: ./.github/actions/setup-node-pnpm
|
||||
@@ -43,6 +46,102 @@ jobs:
|
||||
PR_TITLE: ${{ github.event.pull_request.title }}
|
||||
run: printf '%s\n' "$PR_TITLE" | pnpm exec commitlint --config commitlint.config.cjs
|
||||
|
||||
- name: Lint PR commit headers
|
||||
shell: bash
|
||||
env:
|
||||
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: pnpm exec commitlint --config commitlint.config.cjs --from "$PR_BASE_SHA" --to "$PR_HEAD_SHA" --verbose
|
||||
|
||||
- name: Validate referenced issues exist
|
||||
shell: bash
|
||||
env:
|
||||
GH_REPO: ${{ github.repository }}
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
PR_BRANCH: ${{ github.head_ref }}
|
||||
PR_BODY: ${{ github.event.pull_request.body }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
validate_issue_exists() {
|
||||
local issue_repo="$1"
|
||||
local issue_number="$2"
|
||||
local reference_context="$3"
|
||||
local response_file
|
||||
local http_code
|
||||
local api_url
|
||||
local state
|
||||
local reference_label
|
||||
|
||||
response_file="$(mktemp)"
|
||||
api_url="https://api.github.com/repos/${issue_repo}/issues/${issue_number}"
|
||||
http_code="$(curl -sS -o "${response_file}" -w "%{http_code}" \
|
||||
-H "Authorization: Bearer ${GH_TOKEN}" \
|
||||
-H "Accept: application/vnd.github+json" \
|
||||
"${api_url}")"
|
||||
|
||||
if [[ "${issue_repo}" == "${GH_REPO}" ]]; then
|
||||
reference_label="#${issue_number}"
|
||||
else
|
||||
reference_label="${issue_repo}#${issue_number}"
|
||||
fi
|
||||
|
||||
if [[ "${http_code}" != "200" ]]; then
|
||||
echo "::error::${reference_context} references ${reference_label}, but it does not exist or is not accessible."
|
||||
rm -f "${response_file}"
|
||||
return 1
|
||||
fi
|
||||
|
||||
if jq -e '.pull_request != null' "${response_file}" > /dev/null; then
|
||||
echo "::error::${reference_context} references ${reference_label}, but that number is a pull request, not an issue."
|
||||
rm -f "${response_file}"
|
||||
return 1
|
||||
fi
|
||||
|
||||
state="$(jq -r '.state // "unknown"' "${response_file}")"
|
||||
echo "${reference_context}: issue ${reference_label} exists (state=${state})."
|
||||
rm -f "${response_file}"
|
||||
}
|
||||
|
||||
if [[ "${PR_BRANCH}" =~ ^BO-([0-9]+)- ]]; then
|
||||
branch_issue_number="${BASH_REMATCH[1]}"
|
||||
else
|
||||
echo "::error::Unable to extract issue number from branch '${PR_BRANCH}'."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
validate_issue_exists "${GH_REPO}" "${branch_issue_number}" "Branch name"
|
||||
|
||||
closing_keyword_pattern='(close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved)'
|
||||
issue_reference_pattern="(^|[^[:alnum:]_])${closing_keyword_pattern}([[:space:]]*:[[:space:]]*|[[:space:]]+)([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#[1-9][0-9]*"
|
||||
issue_matches="$(printf '%s\n' "${PR_BODY}" | grep -Eio "${issue_reference_pattern}" || true)"
|
||||
if [[ -z "${issue_matches}" ]]; then
|
||||
echo "::error::PR description must include at least one GitHub closing-keyword issue reference (for example: 'Closes #123', 'Fixes: #123', or 'RESOLVED owner/repo#123')."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
issue_refs="$(printf '%s\n' "${issue_matches}" \
|
||||
| tr '[:upper:]' '[:lower:]' \
|
||||
| sed -E 's/^[^a-z0-9_]*//' \
|
||||
| sed -E "s/^${closing_keyword_pattern}([[:space:]]*:[[:space:]]*|[[:space:]]+)//" \
|
||||
| sort -u)"
|
||||
while IFS= read -r issue_ref; do
|
||||
[[ -z "${issue_ref}" ]] && continue
|
||||
|
||||
if [[ "${issue_ref}" =~ ^([a-z0-9_.-]+/[a-z0-9_.-]+)#([1-9][0-9]*)$ ]]; then
|
||||
issue_repo="${BASH_REMATCH[1]}"
|
||||
issue_number="${BASH_REMATCH[2]}"
|
||||
elif [[ "${issue_ref}" =~ ^#([1-9][0-9]*)$ ]]; then
|
||||
issue_repo="${GH_REPO}"
|
||||
issue_number="${BASH_REMATCH[1]}"
|
||||
else
|
||||
echo "::error::Unable to parse issue reference '${issue_ref}' in PR description."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
validate_issue_exists "${issue_repo}" "${issue_number}" "PR description"
|
||||
done <<< "${issue_refs}"
|
||||
|
||||
release-dry-run:
|
||||
name: Release Dry Run
|
||||
if: github.event_name == 'pull_request'
|
||||
|
||||
@@ -2,6 +2,8 @@ module.exports = {
|
||||
extends: ["@commitlint/config-conventional"],
|
||||
rules: {
|
||||
"type-enum": [2, "always", ["feat", "fix", "db", "perf", "refactor", "style", "docs", "test", "build", "ci", "chore", "security", "revert"]],
|
||||
"header-max-length": [2, "always", 72],
|
||||
"subject-full-stop": [2, "never", "."],
|
||||
"scope-enum": [
|
||||
2,
|
||||
"always",
|
||||
|
||||
+5
-5
@@ -1,12 +1,12 @@
|
||||
# AI Usage Policy
|
||||
|
||||
BookOrbit is maintained by real people. Every pull request, issue, and discussion is read and reviewed by humans who give their time to this project. Submitting unreviewed, untested AI output shifts the burden of that review onto maintainers - that is the problem this policy addresses. It is not a problem with AI tools themselves.
|
||||
BookOrbit is maintained by real people. Every pull request, issue, and discussion is read and reviewed by humans who give their time to this project. Submitting unreviewed, untested AI output shifts the burden of that review onto maintainers; this is the problem this policy addresses. It is not a problem with AI tools themselves.
|
||||
|
||||
## The Rules
|
||||
|
||||
- **All AI use must be disclosed.** If any part of your contribution was written, generated, or significantly shaped by an AI tool, say so in the PR description. There are no exceptions and no minimum threshold. See [Disclosure Format](#disclosure-format) below.
|
||||
|
||||
- **You must understand every line you submit.** If you cannot explain what a piece of code does, why it is structured the way it is, and how it fits into the surrounding system - do not submit it. "The AI wrote it" is not an answer during review.
|
||||
- **You must understand every line you submit.** If you cannot explain what a piece of code does, why it is structured the way it is, and how it fits into the surrounding system, do not submit it. "The AI wrote it" is not an answer during review.
|
||||
|
||||
- **AI-generated tests must validate real behavior.** Tests that mock away the logic under test, assert on implementation details instead of outcomes, or trivially pass regardless of what the code does will be rejected. Read every assertion before submitting.
|
||||
|
||||
@@ -27,7 +27,7 @@ Be honest. A brief, accurate note is better than a vague one that obscures how m
|
||||
|
||||
## Consequences
|
||||
|
||||
PRs that contain undisclosed AI use or that are clearly unreviewed AI output will be closed immediately with a short explanation. There is no detailed feedback given for this - that burden belongs to the contributor, not the reviewer.
|
||||
PRs that contain undisclosed AI use or that are clearly unreviewed AI output will be closed immediately with a short explanation. No detailed feedback is provided for these cases; that burden belongs to the contributor, not the reviewer.
|
||||
|
||||
You are always welcome to resubmit once the work is genuinely reviewed, tested, and properly disclosed. The door is not closed permanently.
|
||||
|
||||
@@ -35,7 +35,7 @@ You are always welcome to resubmit once the work is genuinely reviewed, tested,
|
||||
|
||||
AI tools make specific, recurring mistakes on this codebase. Be especially careful here.
|
||||
|
||||
**Drizzle ORM hallucinations.** LLMs are trained heavily on Prisma and will generate Prisma-style calls - `.findMany()`, `.findUnique()`, `.create()` - that do not exist in Drizzle. Drizzle uses a query builder. Verify every database call against the [Drizzle documentation](https://orm.drizzle.team/docs/overview) and against how the rest of the codebase does it.
|
||||
**Drizzle ORM hallucinations.** LLMs are trained heavily on Prisma and will generate Prisma-style calls such as `.findMany()`, `.findUnique()`, and `.create()` that do not exist in Drizzle. Drizzle uses a query builder. Verify every database call against the [Drizzle documentation](https://orm.drizzle.team/docs/overview) and against how the rest of the codebase does it.
|
||||
|
||||
**NestJS decorator and module mistakes.** AI frequently invents decorator options, misuses pipes, omits required module imports, or generates module wiring that looks plausible but is wrong. Check every decorator and every module registration.
|
||||
|
||||
@@ -64,7 +64,7 @@ These rules apply to outside contributors. Maintainers are exempt. They have dem
|
||||
|
||||
To be clear: this policy is not an anti-AI stance. Maintainers use AI tools. Many parts of this codebase were written with AI assistance.
|
||||
|
||||
The problem this policy addresses is unqualified, unreviewed output being submitted as a contribution - work that the contributor has not run, does not understand, and cannot stand behind. That is a burden on maintainers regardless of whether it came from a human or a machine.
|
||||
The problem this policy addresses is unqualified, unreviewed output being submitted as a contribution: work that the contributor has not run, does not understand, and cannot stand behind. That is a burden on maintainers regardless of whether it came from a human or a machine.
|
||||
|
||||
Use the tools. Do the work.
|
||||
|
||||
|
||||
@@ -59,8 +59,11 @@ appointed representative at an online or offline event.
|
||||
## Enforcement
|
||||
|
||||
Instances of abusive, harassing, or otherwise unacceptable behavior may be
|
||||
reported to the community leaders responsible for enforcement at [INSERT CONTACT
|
||||
METHOD]. All complaints will be reviewed and investigated promptly and fairly.
|
||||
reported to the community leaders via GitHub's private reporting channel:
|
||||
https://github.com/bookorbit/bookorbit/security/advisories/new. For conduct
|
||||
reports, include "Code of Conduct report" in the title so it can be triaged
|
||||
appropriately. All complaints will be reviewed and investigated promptly and
|
||||
fairly.
|
||||
|
||||
All community leaders are obligated to respect the privacy and security of the
|
||||
reporter of any incident.
|
||||
|
||||
+18
-13
@@ -14,7 +14,7 @@ Every commit tells a story. A well-written commit message makes it easy to under
|
||||
|
||||
The **header** is required on every commit and must follow the format described below.
|
||||
|
||||
The **body** is required on all commits except `docs`. When present, it must be at least 20 characters and written in imperative mood.
|
||||
The **body** is required on all commits except `docs`. When present, it must be at least 20 characters and written in the imperative mood.
|
||||
|
||||
The **footer** is optional. Use it to reference issues, close PRs, or document breaking changes.
|
||||
|
||||
@@ -52,12 +52,12 @@ The type signals the intent of the change at a glance.
|
||||
| `docs` | Documentation changes only |
|
||||
| `test` | Adds or corrects tests without touching production code |
|
||||
| `build` | Changes to the build system, tooling config, or external dependencies (Dockerfile, tsconfig, pnpm) |
|
||||
| `ci` | CI/CD pipeline changes - GitHub Actions workflows and scripts |
|
||||
| `chore` | Routine maintenance that doesn't fit any other category |
|
||||
| `ci` | CI/CD pipeline changes: GitHub Actions workflows and scripts |
|
||||
| `chore` | Routine maintenance that does not fit any other category |
|
||||
| `security` | Addresses a security vulnerability; preferred over `fix` so security patches are identifiable in history |
|
||||
| `revert` | Reverts a previous commit - the body must reference the reverted SHA and explain why |
|
||||
| `revert` | Reverts a previous commit; the body must reference the reverted SHA and explain why |
|
||||
|
||||
> If a change genuinely spans multiple types (for example, it adds a feature and fixes a bug), split it into two commits. If that isn't practical, use the type that best represents the dominant intent.
|
||||
> If a change genuinely spans multiple types (for example, it adds a feature and fixes a bug), split it into two commits. If that is not practical, use the type that best represents the dominant intent.
|
||||
|
||||
### Scope
|
||||
|
||||
@@ -109,13 +109,13 @@ Rules:
|
||||
|
||||
## Body
|
||||
|
||||
Write a body when the **why** behind the change isn't obvious from the header alone.
|
||||
Write a body when the **why** behind the change is not obvious from the header alone.
|
||||
|
||||
When to write a body:
|
||||
|
||||
- The motivation isn't clear from the summary
|
||||
- The motivation is not clear from the summary
|
||||
- The change has side effects, tradeoffs, or gotchas worth calling out
|
||||
- You're reverting a commit - reference the SHA and explain the reason
|
||||
- You are reverting a commit; reference the SHA and explain the reason
|
||||
- The approach taken deserves a brief explanation
|
||||
|
||||
Format:
|
||||
@@ -134,12 +134,17 @@ Use the footer to link related issues or PRs, or to announce breaking changes.
|
||||
### Linking issues and PRs
|
||||
|
||||
```
|
||||
Fixes #123
|
||||
Closes #456
|
||||
Ref #789
|
||||
Closes #123
|
||||
Fixes: #456
|
||||
RESOLVED owner/repo#789
|
||||
Ref #101
|
||||
```
|
||||
|
||||
`Fixes` and `Closes` automatically close the linked issue when the PR is merged. Use `Ref` for a related issue that should stay open.
|
||||
GitHub-supported closing keywords are: `close`, `closes`, `closed`, `fix`, `fixes`, `fixed`, `resolve`, `resolves`, `resolved` (case-insensitive; optional colon is allowed, for example `Closes: #123`).
|
||||
|
||||
Use `Ref #<issue>` for a related issue that should stay open.
|
||||
|
||||
`Ref` is valid for commit footers, but it does not satisfy the PR-description requirement in CI (PR descriptions must include a GitHub closing keyword).
|
||||
|
||||
### Breaking changes
|
||||
|
||||
@@ -168,7 +173,7 @@ feat(kobo): sync reading position on device reconnect
|
||||
feat(opds): add pagination to catalog feed endpoints
|
||||
|
||||
The OPDS feed now supports ?page= and ?limit= query params.
|
||||
Default page size is 20. Clients that don't send pagination params
|
||||
Default page size is 20. Clients that do not send pagination params
|
||||
get the same behavior as before.
|
||||
|
||||
Closes #301
|
||||
|
||||
+21
-14
@@ -24,7 +24,7 @@ If nothing there catches your eye, browse the open issues or propose something n
|
||||
Doc improvements are always welcome and have a lighter process:
|
||||
|
||||
- No tests required.
|
||||
- Use the `docs/` branch prefix (e.g. `docs/fix-setup-instructions`).
|
||||
- Use a branch name that starts with `BO-<issue-number>-` (e.g. `BO-456-docs-fix-setup-instructions`).
|
||||
- Focus areas: fixing inaccuracies, improving examples, adding missing guides, clarifying confusing sections.
|
||||
|
||||
The rest of this guide still applies (issue first, PR template, etc.), but expect a faster review cycle.
|
||||
@@ -33,7 +33,7 @@ The rest of this guide still applies (issue first, PR template, etc.), but expec
|
||||
|
||||
## Security Issues
|
||||
|
||||
Do **not** open public issues for security vulnerabilities. Use GitHub's [private vulnerability reporting](../../security/advisories/new) instead. See [SECURITY.md](.github/SECURITY.md) for details.
|
||||
Do **not** open public issues for security vulnerabilities. Use GitHub's [private vulnerability reporting](../../security/advisories/new) instead. See [SECURITY.md](../.github/SECURITY.md) for details.
|
||||
|
||||
---
|
||||
|
||||
@@ -54,7 +54,7 @@ This is the end-to-end journey from idea to merged PR. Each phase builds on the
|
||||
Wait for explicit approval before writing code. Approval looks like:
|
||||
|
||||
- A :+1: reaction on your issue.
|
||||
- A comment such as "go ahead", "approved", or "let's do it".
|
||||
- A comment such as "go ahead", "approved", or "approved to proceed".
|
||||
|
||||
This step prevents wasted effort on changes that conflict with the project direction. Small bug fixes with clear reproduction are usually approved quickly.
|
||||
|
||||
@@ -86,18 +86,14 @@ Always branch from the latest `main`:
|
||||
|
||||
```bash
|
||||
git fetch upstream
|
||||
git checkout -b <prefix>/<short-description> upstream/main
|
||||
git checkout -b BO-<issue-number>-<short-description> upstream/main
|
||||
```
|
||||
|
||||
Use these branch prefixes:
|
||||
Branch name requirements:
|
||||
|
||||
| Prefix | Use for | Example |
|
||||
| ----------- | -------------------------------------- | -------------------------------- |
|
||||
| `feat/` | New features | `feat/reading-progress-sync` |
|
||||
| `fix/` | Bug fixes | `fix/pdf-page-count-off-by-one` |
|
||||
| `refactor/` | Code restructuring | `refactor/library-scan-pipeline` |
|
||||
| `docs/` | Documentation | `docs/opds-setup-guide` |
|
||||
| `chore/` | Maintenance (deps, tooling, CI, build) | `chore/bump-drizzle-v1` |
|
||||
- Start with `BO-<issue-number>-`
|
||||
- Use lowercase kebab-case for the description suffix
|
||||
- Example: `BO-123-fix-reader-sync`
|
||||
|
||||
### Phase 6: Implement Your Change
|
||||
|
||||
@@ -169,13 +165,24 @@ Examples: `feat(kobo): add shelf sync endpoint`, `fix(reader): correct page coun
|
||||
Push your branch and open a PR against `main`:
|
||||
|
||||
```bash
|
||||
git push origin feat/your-feature-name
|
||||
git push origin BO-123-your-feature-name
|
||||
```
|
||||
|
||||
When creating the PR:
|
||||
|
||||
- **Fill out the PR template.** It asks for a summary, testing evidence, screenshots (if UI changed), and non-obvious decisions.
|
||||
- **Link your issue** with `Closes #123` in the description.
|
||||
- **Use a Conventional Commit-style PR title** (for example: `fix(reader): correct page count for multi-volume PDFs`).
|
||||
- **Link your issue** with a GitHub closing keyword in the description (`close`, `closes`, `closed`, `fix`, `fixes`, `fixed`, `resolve`, `resolves`, `resolved`) and an issue reference (`#123` or `owner/repo#123`).
|
||||
Example:
|
||||
|
||||
```md
|
||||
## What does this PR do?
|
||||
|
||||
Correct page count handling for multi-volume PDFs.
|
||||
|
||||
Fixes: #123
|
||||
```
|
||||
|
||||
- **Open as a draft** if you want early direction feedback before the code is complete.
|
||||
- **Disclose AI usage** if applicable. See [AI_POLICY.md](AI_POLICY.md) for the format and expectations.
|
||||
|
||||
|
||||
+20
-10
@@ -292,7 +292,7 @@ pnpm run db:seed
|
||||
| `pnpm run e2e:list` | List available E2E suite IDs |
|
||||
| `pnpm run coverage` | Unit tests with coverage reports |
|
||||
|
||||
Server unit tests use Vitest with `@nestjs/testing`. Client tests use Vitest with `@vue/test-utils` in a jsdom environment. Most E2E suites boot the full app against a dedicated test database; a few smoke-style suites run against the dev database.
|
||||
Server unit tests use Vitest with `@nestjs/testing`. Client tests use Vitest with `@vue/test-utils` in a jsdom environment. Most E2E suites boot the full app against a dedicated test database; a few smoke-style suites run against a shared E2E database without a reset.
|
||||
|
||||
For test architecture, suite details, coverage thresholds, the E2E harness, and examples, see [TESTING.md](TESTING.md).
|
||||
|
||||
@@ -353,12 +353,12 @@ These are the rules that most commonly cause PR rejections. Following them from
|
||||
|
||||
The CI workflow (`.github/workflows/ci.yml`) triggers on every PR to `main`:
|
||||
|
||||
1. **Dependency review** - Checks for supply chain issues in new dependencies.
|
||||
2. **Change detection** - Determines which workspaces (server/client) have changed files.
|
||||
3. **Lint** - Runs ESLint and format checks on changed workspaces.
|
||||
4. **Type check** - Runs TypeScript checking on changed workspaces.
|
||||
5. **Tests** - Runs unit tests on changed workspaces with coverage.
|
||||
6. **E2E** - Selects and runs relevant E2E suites based on changed file paths.
|
||||
1. **PR conventions** - Enforces `BO-<issue-number>-<short-description>` branch names, PR title format, commit-header rules from [COMMIT_GUIDELINES.md](COMMIT_GUIDELINES.md), and a PR-description issue link using GitHub closing keywords (`close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved`). CI also verifies that referenced issue numbers actually exist.
|
||||
2. **Dependency review** - Checks for supply chain issues in new dependencies.
|
||||
3. **Change detection** - Determines which workspaces (server/client) have changed files.
|
||||
4. **Lint** - Runs ESLint and format checks on changed workspaces.
|
||||
5. **Type check** - Runs TypeScript checking on changed workspaces.
|
||||
6. **Tests** - Runs unit tests on changed workspaces with coverage.
|
||||
|
||||
All jobs are conditional: if you only changed client code, server lint/typecheck/tests are skipped.
|
||||
|
||||
@@ -377,9 +377,19 @@ Releases are created manually via `workflow_dispatch` on the `release.yml` workf
|
||||
3. Creates a Git tag and GitHub Release with auto-generated release notes.
|
||||
4. Builds, scans, and publishes a Docker image with version and `latest` tags.
|
||||
|
||||
PR titles are validated by commitlint to ensure they follow the [commit guidelines](COMMIT_GUIDELINES.md). Only releasable types (`feat`, `fix`, `perf`, `security`, `db`, `style`) trigger version bumps.
|
||||
On pull requests, CI enforces branch naming (`BO-<issue-number>-<short-description>`), validates PR titles with commitlint (for squash-merge release safety), validates PR commit headers against [COMMIT_GUIDELINES.md](COMMIT_GUIDELINES.md), and requires at least one GitHub closing-keyword issue reference in the PR description (for example: `Closes #123`, `Fixes: #123`, or `RESOLVED owner/repo#123`). It also verifies that the branch issue exists in this repository and that PR description issue references resolve to valid GitHub issues. Only releasable types (`feat`, `fix`, `perf`, `security`, `db`, `style`) trigger version bumps.
|
||||
|
||||
For the full release runbook, see [`docs/RELEASE_PROCESS.md`](docs/RELEASE_PROCESS.md).
|
||||
Example PR description snippet:
|
||||
|
||||
```md
|
||||
## What does this PR do?
|
||||
|
||||
Adds pagination for OPDS search results.
|
||||
|
||||
Closes #321
|
||||
```
|
||||
|
||||
For the full release runbook, see [`RELEASE_PROCESS.md`](RELEASE_PROCESS.md).
|
||||
|
||||
### E2E in CI
|
||||
|
||||
@@ -445,7 +455,7 @@ Restart the dev server. If you changed `package.json` or installed new dependenc
|
||||
|
||||
**Docker volumes corrupted**
|
||||
|
||||
Nuclear option. This destroys all local data. Stop the container with `-v` to drop volumes, then rebuild:
|
||||
Last resort. This destroys all local data. Stop the container with `-v` to drop volumes, then rebuild:
|
||||
|
||||
```bash
|
||||
docker compose -f docker-compose.dev.yml down -v
|
||||
|
||||
+39
-5
@@ -20,15 +20,49 @@ BookOrbit uses [semantic-release](https://github.com/semantic-release/semantic-r
|
||||
|
||||
All PRs to `main` must use **squash merge**. The squash commit message becomes the commit that semantic-release analyzes for version bumps.
|
||||
|
||||
### PR title format
|
||||
### PR title, commit message, and issue link format
|
||||
|
||||
PR titles must follow the [commit guidelines](COMMIT_GUIDELINES.md):
|
||||
PR titles and all commits in a PR must follow the [commit guidelines](COMMIT_GUIDELINES.md):
|
||||
|
||||
```
|
||||
<type>(<scope>): <summary>
|
||||
```
|
||||
|
||||
The PR title is validated by commitlint in the release workflow. Invalid titles will fail the `Lint PR Title` check.
|
||||
Why PR title is required: with squash merges, the final commit analyzed by semantic-release is based on the PR title.
|
||||
|
||||
Validation in CI and release checks:
|
||||
|
||||
- PR branch name must follow `BO-123-fix-reader-pagination`
|
||||
- PR title must match commitlint rules
|
||||
- Example: `fix(reader): reduce rerendering in chapter list`
|
||||
- PR commit headers must match commitlint rules
|
||||
- PR description must include at least one GitHub closing-keyword issue reference:
|
||||
- `close`, `closes`, `closed`, `fix`, `fixes`, `fixed`, `resolve`, `resolves`, `resolved`
|
||||
- Examples: `Closes #123`, `Fixes: #123`, `RESOLVED owner/repo#123`
|
||||
- Branch issue number must exist in this repository
|
||||
- PR description issue references must resolve to valid GitHub issues (same repo or `owner/repo#issue`)
|
||||
|
||||
Examples:
|
||||
|
||||
- Valid:
|
||||
|
||||
```md
|
||||
## What does this PR do?
|
||||
|
||||
Improve reader performance when opening large EPUB files.
|
||||
|
||||
Fixes #145
|
||||
```
|
||||
|
||||
- Invalid (fails lint):
|
||||
|
||||
```md
|
||||
## What does this PR do?
|
||||
|
||||
Improve reader performance when opening large EPUB files.
|
||||
|
||||
fixes issue 145
|
||||
```
|
||||
|
||||
### Types that trigger releases
|
||||
|
||||
@@ -145,7 +179,7 @@ The `release.yml` workflow has four jobs:
|
||||
└──────────────┘ └──────────────────┘
|
||||
```
|
||||
|
||||
- **commitlint**: Validates PR title format (PR only).
|
||||
- **commitlint**: Validates branch naming, PR title, commit headers, and PR description issue-link and issue-existence rules (PR only).
|
||||
- **release-dry-run**: Runs semantic-release in dry-run mode to validate config (PR only).
|
||||
- **release**: Creates the actual release (manual dispatch only). Outputs `published`, `version`, and `tag`.
|
||||
- **docker-publish**: Builds and pushes Docker image (only when a release was published).
|
||||
@@ -162,6 +196,6 @@ The `release.yml` workflow has four jobs:
|
||||
|
||||
| File | Purpose |
|
||||
| ------------------------------- | ------------------------------------------------ |
|
||||
| `.releaserc.json` | semantic-release config (plugins, release rules) |
|
||||
| `release.config.js` | semantic-release config (plugins, release rules) |
|
||||
| `commitlint.config.cjs` | commitlint config (type/scope validation) |
|
||||
| `.github/workflows/release.yml` | Release and Docker publish workflow |
|
||||
|
||||
+3
-3
@@ -6,7 +6,7 @@ BookOrbit has three layers of tests:
|
||||
- **Client unit tests:** tests for Vue composables and pure utility functions using a jsdom environment.
|
||||
- **End-to-end (E2E) tests:** full-stack tests that boot a real NestJS application and run HTTP requests against a PostgreSQL database.
|
||||
|
||||
Each layer has a specific job and its own tooling. Don't cover the same thing at multiple layers.
|
||||
Each layer has a specific job and its own tooling. Do not test the same behavior at multiple layers.
|
||||
|
||||
For testing expectations by change type, see [CONTRIBUTING.md](CONTRIBUTING.md).
|
||||
For the development setup, see [DEVELOPMENT.md](DEVELOPMENT.md).
|
||||
@@ -82,7 +82,7 @@ Every suite runs in one of two modes:
|
||||
|
||||
| Mode | `prepareDedicatedDatabase` | Database used | When to use |
|
||||
| ------------ | -------------------------- | ------------------------------------------------------- | ------------------------------------------------ |
|
||||
| default-db | `false` | `E2E_DATABASE_URL` with no reset before run | Fast smoke tests that don't write data |
|
||||
| default-db | `false` | `E2E_DATABASE_URL` with no reset before run | Fast smoke tests that do not write data |
|
||||
| dedicated-db | `true` | `bookorbit_e2e` dropped and re-migrated before each run | Any test that writes data or needs a clean state |
|
||||
|
||||
For dedicated-db suites, `run-suite.mjs` automatically runs `pnpm run e2e:db:prepare` before
|
||||
@@ -184,7 +184,7 @@ same global pipes and exception filter as production, registers cookie support,
|
||||
| `ctx.adminToken` | `string` | A valid JWT for the seeded superuser |
|
||||
|
||||
`MetadataService` is replaced with a no-op mock by default so E2E tests never make external
|
||||
metadata provider calls. To customise the mock for a specific suite, pass the result of
|
||||
metadata provider calls. To customize the mock for a specific suite, pass the result of
|
||||
`makeMetadataNoopMock()` and configure its `vi.fn()` methods before calling
|
||||
`createE2EContext()`.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user