diff --git a/.github/PULL_REQUEST_TEMPLATE b/.github/PULL_REQUEST_TEMPLATE index 41287564ff3..10248925cbd 100644 --- a/.github/PULL_REQUEST_TEMPLATE +++ b/.github/PULL_REQUEST_TEMPLATE @@ -4,8 +4,20 @@ Thanks for sending a pull request (PR)! Here are some tips for you: [Contributing to Texera](https://github.com/apache/texera/blob/main/CONTRIBUTING.md) 2. Ensure you have added or run the appropriate tests for your PR 3. If the PR is work in progress, mark it a draft on GitHub. - 4. Please write your PR title to summarize what this PR proposes, we - are following Conventional Commits style for PR titles as well. + 4. Please write your PR title to summarize what this PR proposes, we + are following Conventional Commits style for PR titles as well: + - `fix` is for behavior that worked before and no longer does; adding or + removing a functionality, or reworking one so that user-facing behavior + intentionally changes, is a `feat`; a change that leaves the user-facing + behavior unchanged is a `refactor`. + - A test-only PR is `test(): ...`; repairing a broken test is + `fix(test, ): ...`. + - A dependency bump is `fix(deps, ): ...` when it patches a CVE + and `chore(deps, ): ...` otherwise; GitHub Actions bumps take + `ci` as their module, e.g. `chore(deps, ci): ...`. + - A PR targeting a release branch appends the version as the last scope + component, e.g. `fix(deps, frontend, v1.2): ...`. + See CONTRIBUTING.md for the full convention. 5. Be sure to keep the PR description updated to reflect all changes. --> diff --git a/AGENTS.md b/AGENTS.md index 7118073a781..c94002c86a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -138,18 +138,52 @@ Short, **Conventional Commits**, same shape for branch and commit subject. | Feature | `feat/agent-workflow-edit` | `feat(agent-service): enable workflow edit` | | Bug fix | `fix/marker-replay` | `fix(amber): marker replay during reconfiguration` | | Tests | `test/pyamber-handlers` | `test(pyamber): add handler unit tests` | -| Chore | `chore/angular-21` | `chore(deps): upgrade frontend to Angular 21` | -| CI | `ci/cache-action-bump` | `ci: bump coursier/cache-action to v8.1.0` | +| Chore | `chore/angular-21` | `chore(deps, frontend): upgrade to Angular 21` | +| CI | `ci/merge-queue-stacking` | `ci: stack merge-queue builds by module` | Both ≤ ~60 chars. For code changes, if you use a scope, use the module name (`amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, …) — not -`amber-python`. Dependency-only updates split by semantics: `fix(deps): ...` for -runtime/production dependency bumps (they ship to users), -`chore(deps): ...` for dev/toolchain-only bumps, and `ci: ...` for -CI-only changes (including GitHub Actions bumps). Append the module -as a second scope when the bump is module-specific, e.g. -`fix(deps, pyamber): ...`; omit it for cross-module bumps (sbt). No `Co-authored-by:` trailer for the repo -owner. +`amber-python`. No `Co-authored-by:` trailer for the repo owner. + +**Choosing the type** turns on what happens to the behavior, not on how big +the diff is: + +| The change | Type | +| --- | --- | +| Worked before, broken now | `fix` | +| Support never existed; adding it | `feat` | +| Support exists; removing it | `feat` | +| Reworked so user-facing behavior intentionally changes | `feat` | +| User-facing behavior unchanged | `refactor` | + +Behavior is what the code does, not what a doc or an old PR description claims +it does: implementing something that was never actually there is a `feat`. + +`refactor` claims the **user-facing** behavior is identical. Tests that pin a +user-facing API must pass untouched — editing one of those assertions means +the behavior moved, so it is a `feat` or a `fix`. Tests that pin internals (a +private helper's signature, call order between collaborators, the shape of an +intermediate value) mirror the implementation, so rewriting them alongside the +code they mirror is still a `refactor`. + +**Tests.** A test-only PR is `test(): ...`. Repairing a broken or +flaky test is a bug fix in test code: `fix(test, ): ...`. + +**Dependencies.** `fix` only when the bump carries a security fix: + +| Bump | Commit | +| --- | --- | +| Patches a CVE | `fix(deps, ): ...` | +| Everything else | `chore(deps, ): ...` | +| GitHub Actions | `chore(deps, ci): ...` | + +Omit the module for cross-module bumps (sbt). GitHub Actions bumps take `ci` +as their module — that is what [`.github/renovate.json5`](.github/renovate.json5) +opens them with; a bare `ci: ...` is for hand-written CI and workflow changes. + +**Backports.** A PR targeting `release/vX.Y` appends the version as the last +scope component — `fix(deps, frontend, v1.2): ...`. Version tags belong only +on release-branch PRs, never on one targeting `main`. ### Issues and PRs @@ -193,7 +227,7 @@ write/adjust test (red) -> edit source (green) -> refactor | New feature / behavior change | Failing test, then implement. | | Bug fix | Regression test reproducing the bug, then fix. | | Code with **no tests** | **Characterization tests** pin current behavior first; only then change source. | -| Refactor (no behavior change) | Tests stay green throughout — no assertion edits. | +| Refactor (no user-facing behavior change) | Tests stay green throughout. User-facing API assertions stay untouched; tests that mirror internals may be rewritten with the code. | Every test must cover: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7557eb7a286..99e3e009886 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,8 +51,43 @@ Thank you for your interest in contributing to Texera! Please follow the steps b - All PR titles will be used as the **squashed commit message** when merged into the `main` branch. - Example PR titles: - `feat: add a new join operator` - - `fix(ui): prevent racing of requests` - - `chore(deps): bump numpy to version 2.0.0` + - `fix(frontend): prevent racing of requests` + - `chore(deps, pyamber): bump numpy to version 2.0.0` + +A scope names the module the change lands in — `amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, and so on. Use the module's own name rather than an informal synonym, and when a PR spans modules, scope it to the one carrying the substantive change. + +##### Choosing between `feat`, `fix`, and `refactor` + +The type depends on what happens to the behavior, not on how large the change is. + +| Your change | Type | +| ----------- | ---- | +| A functionality worked before and no longer does | `fix` | +| A functionality or a form of support never existed and you are adding it | `feat` | +| A functionality exists and you are removing support for it | `feat` | +| A functionality is reworked in a way that intentionally changes user-facing behavior | `feat` | +| The change leaves the user-facing behavior unchanged | `refactor` | + +Behavior is defined by the code, not by what a document or an old PR description says the code does. A functionality that was never implemented does not exist, so implementing it is a `feat` even when the docs already described it as present. + +`refactor` is a strong claim: it says the **user-facing** behavior is identical. The test suite is how you check that, but not every test carries the same weight. A test that pins a user-facing API is the contract — if you had to change one of its assertions to make the suite green, the behavior moved, and the PR is a `feat` or a `fix`. A test that pins internals, such as a private helper's signature, the call order between two collaborators, or the shape of an intermediate value, is mirroring the implementation; rewriting it alongside the code it mirrors is expected and still a `refactor`. + +##### Tests and dependency bumps + +Test and dependency PRs take the titles below. Where the table shows a two-part scope, it is written as `(, ): `: + +| Your change | Title | +| ----------- | ----- | +| A test-only PR — adding or updating tests | `test(): ...`, e.g. `test(amber): add marker replay specs` | +| Repairing a broken or flaky test | `fix(test, ): ...`, e.g. `fix(test, frontend): stabilize the dashboard spec` | +| A dependency bump that patches a CVE | `fix(deps, ): ...`, e.g. `fix(deps, pyamber): bump protobuf for CVE-2025-4565` | +| Any other dependency bump | `chore(deps, ): ...`, e.g. `chore(deps, pyamber): bump numpy to 2.0.0` | + +Omit the module for bumps that span modules. GitHub Actions bumps are dependency bumps too and take the `ci` module — `chore(deps, ci): ...`, or `fix(deps, ci): ...` when the bump patches a CVE — which is the form [`.github/renovate.json5`](.github/renovate.json5) opens them with. Reserve a bare `ci: ...` for hand-written CI and workflow changes. + +##### Backports + +A PR targeting a release branch appends the release version as the **last scope component**, so a backport of `fix(deps, frontend): ...` to `release/v1.2` is titled `fix(deps, frontend, v1.2): ...`. Version tags belong only on release-branch PRs — never put one on a PR targeting `main`. > 💡 You can use the [Conventional Commits plugin](https://plugins.jetbrains.com/plugin/13389-conventional-commit) in IntelliJ to help format commit messages correctly. diff --git a/docs/contribution-guidelines/_index.md b/docs/contribution-guidelines/_index.md index eb062a75295..007f9efa9c9 100644 --- a/docs/contribution-guidelines/_index.md +++ b/docs/contribution-guidelines/_index.md @@ -68,10 +68,39 @@ Fork the [Texera repository](https://github.com/apache/texera) on GitHub and clo We use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/): - Example PR titles: - `feat: add new join operator` - - `fix(ui): resolve workflow panel crash` + - `fix(frontend): resolve workflow panel crash` - `chore(deps): bump dependency versions` - The PR title becomes the final squashed commit message upon merge. +A scope names the module the change lands in — `amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, and so on. Use the module's own name rather than an informal synonym, and when a PR spans modules, scope it to the one carrying the substantive change. + +Pick the type by what happens to the behavior, not by how large the change is: + +| Your change | Type | +| ----------- | ---- | +| A functionality worked before and no longer does | `fix` | +| A functionality or a form of support never existed and you are adding it | `feat` | +| A functionality exists and you are removing support for it | `feat` | +| A functionality is reworked in a way that intentionally changes user-facing behavior | `feat` | +| The change leaves the user-facing behavior unchanged | `refactor` | + +Behavior is defined by the code, not by what a document or an old PR description says the code does. A functionality that was never implemented does not exist, so implementing it is a `feat` even when the docs already described it as present. + +`refactor` claims the **user-facing** behavior is identical. A test that pins a user-facing API must keep passing untouched — changing one of its assertions means the behavior moved, so the PR is a `feat` or a `fix`. A test that pins internals mirrors the implementation and may be rewritten alongside the code it mirrors. + +Test and dependency PRs take the titles below. Where the table shows a two-part scope, it is written as `(, ): `: + +| Your change | Title | +| ----------- | ----- | +| A test-only PR — adding or updating tests | `test(): ...` | +| Repairing a broken or flaky test | `fix(test, ): ...` | +| A dependency bump that patches a CVE | `fix(deps, ): ...` | +| Any other dependency bump | `chore(deps, ): ...` | + +Omit the module for bumps that span modules. GitHub Actions bumps are dependency bumps too and take the `ci` module — `chore(deps, ci): ...`, or `fix(deps, ci): ...` when the bump patches a CVE — which is the form Renovate opens them with. Reserve a bare `ci: ...` for hand-written CI and workflow changes. + +A PR targeting a release branch appends the version as the last scope component — a backport of `fix(deps, frontend): ...` to `release/v1.2` becomes `fix(deps, frontend, v1.2): ...`. Never put a version tag on a PR targeting `main`. + #### PR Description Should Include: - **Purpose:** use `Closes #1234` to auto-close an issue. - **Summary:** short overview of your changes.