-
Notifications
You must be signed in to change notification settings - Fork 0
[replay #7427] docs: define how to pick a PR title's type and scope #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: replay/7427-base
Are you sure you want to change the base?
Changes from all commits
41c8962
2d0c9d0
c0a2292
5cc5ff3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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` | | ||
|
Comment on lines
+63
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Fix the Markdown table style reported by
Proposed Markdown fix-| Your change | Type |
-| ----------- | ---- |
+| Your change | Type
+| ----------- | ----Also applies to: 79-84 🧰 Tools🪛 markdownlint-cli2 (0.23.2)[warning] 63-63: Table pipe style (MD055, table-pipe-style) [warning] 64-64: Table pipe style (MD055, table-pipe-style) [warning] 65-65: Table pipe style (MD055, table-pipe-style) [warning] 66-66: Table pipe style (MD055, table-pipe-style) [warning] 67-67: Table pipe style (MD055, table-pipe-style) [warning] 68-68: Table pipe style (MD055, table-pipe-style) [warning] 69-69: Table pipe style (MD055, table-pipe-style) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| 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 `<type>(<area>, <module>): <description>`: | ||
|
|
||
| | Your change | Title | | ||
| | ----------- | ----- | | ||
| | A test-only PR — adding or updating tests | `test(<module>): ...`, e.g. `test(amber): add marker replay specs` | | ||
| | Repairing a broken or flaky test | `fix(test, <module>): ...`, e.g. `fix(test, frontend): stabilize the dashboard spec` | | ||
| | A dependency bump that patches a CVE | `fix(deps, <module>): ...`, e.g. `fix(deps, pyamber): bump protobuf for CVE-2025-4565` | | ||
| | Any other dependency bump | `chore(deps, <module>): ...`, 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. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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. | ||||||||||||||||||
|
Comment on lines
72
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Clarify the unscoped dependency example. Line 72 shows a bare Proposed documentation fix- - `chore(deps): bump dependency versions`
+ - `chore(deps, pyamber): bump numpy to version 2.0.0`📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| 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 `<type>(<area>, <module>): <description>`: | ||||||||||||||||||
|
|
||||||||||||||||||
| | Your change | Title | | ||||||||||||||||||
| | ----------- | ----- | | ||||||||||||||||||
| | A test-only PR — adding or updating tests | `test(<module>): ...` | | ||||||||||||||||||
| | Repairing a broken or flaky test | `fix(test, <module>): ...` | | ||||||||||||||||||
| | A dependency bump that patches a CVE | `fix(deps, <module>): ...` | | ||||||||||||||||||
| | Any other dependency bump | `chore(deps, <module>): ...` | | ||||||||||||||||||
|
|
||||||||||||||||||
| 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. | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the CVE exception to the GitHub Actions row.
Line 172 says that
fixapplies to security bumps, but Line 178 lists GitHub Actions updates aschore(deps, ci)without an exception. This conflicts withCONTRIBUTING.mdLine 86 anddocs/contribution-guidelines/_index.mdLine 100. A contributor can selectchorefor a CVE-patching Actions update and miss fix-specific backport automation. Add thefix(deps, ci)form for CVE patches.Proposed documentation fix
As per coding guidelines, classify dependency bumps as
fixonly when they patch a CVE and usechoreotherwise.📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Coding guidelines