Skip to content

Commit 515d1bd

Browse files
authored
chore(ci): Add action and documentation for semantic commits (#26122)
## Description We decided in the TSC meeting to use conventional commits to enforce consistent commit messages in this project. This adds an advisory Github Action to validate the PR title, and documentation for developers on the new expectation. When this PR is merged, I will begin to socialize the new requirements over Slack. After a time, we'll make the Github Action mandatory. ## Motivation and Context Consistent commit messages, alignment with Velox and other open source projects ## Impact Presto will begin to use conventional commits. ## Test Plan N/A ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
1 parent fb866da commit 515d1bd

File tree

2 files changed

+140
-49
lines changed

2 files changed

+140
-49
lines changed
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: Semantic Commit Check
2+
3+
on:
4+
pull_request:
5+
types:
6+
- opened
7+
- reopened
8+
- edited
9+
- synchronize
10+
11+
permissions:
12+
contents: read
13+
14+
jobs:
15+
semantic-pr:
16+
concurrency:
17+
group: ${{ github.workflow }}-semantic-commit-check-${{ github.event.pull_request.number }}
18+
cancel-in-progress: true
19+
name: Validate PR Title
20+
runs-on: ubuntu-latest
21+
steps:
22+
- uses: amannn/action-semantic-pull-request@2d952a1bf90a6a7ab8f0293dc86f5fdf9acb1915 # v5.5.3
23+
env:
24+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
25+
with:
26+
types: |
27+
feat
28+
fix
29+
docs
30+
refactor
31+
perf
32+
test
33+
build
34+
ci
35+
chore
36+
revert
37+
misc
38+
requireScope: false
39+
subjectPattern: ^[A-Z].*[^.]$
40+
subjectPatternError: |
41+
The PR title subject must start with a capital letter and not end with a period.

CONTRIBUTING.md

Lines changed: 99 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -310,42 +310,99 @@ We recommend you use IntelliJ as your IDE. The code style template for the proje
310310

311311

312312
## Commit Standards
313-
* ### Commit Size
314-
* Recommended lines of code should be no more than +1000 lines, and should focus on one single major topic.\
315-
If your commit is more than 1000 lines, consider breaking it down into multiple commits, each handling a specific small topic.
316-
* ### Commit Message Style
317-
* **Separate subject from body with a blank line**
318-
* **Subject**
319-
* Limit the subject line to 10 words or 50 characters
320-
* If you cannot make the subject short, you may be committing too many changes at once
321-
* Capitalize the subject line
322-
* Do not end the subject line with a period
323-
* Use the imperative mood in the subject line
324-
* **Body**
325-
* Wrap the body at 72 characters
326-
* Use the body to explain what and why versus how
327-
* Use the indicative mood in the body\
328-
For example, “If applied, this commit will ___________”
329-
* Communicate only context (why) for the commit in the subject line
330-
* Use the body for What and Why
331-
* If your commit is complex or dense, share some of the How context
332-
* Assume someone may need to revert your change during an emergency
333-
* **Content**
334-
* **Aim for smaller commits for easier review and simpler code maintenance**
335-
* All bug fixes and new features must have associated tests
336-
* Commits should pass tests on their own, not be dependent on other commits
337-
* The following is recommended:
338-
* Describe why a change is being made.
339-
* How does it address the issue?
340-
* What effects does the patch have?
341-
* Do not assume the reviewer understands what the original problem was.
342-
* Do not assume the code is self-evident or self-documenting.
343-
* Read the commit message to see if it hints at improved code structure.
344-
* The first commit line is the most important.
345-
* Describe any limitations of the current code.
346-
* Do not include patch set-specific comments.
347-
348-
Details for each point and good commit message examples can be found on https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages
313+
314+
### Conventional Commits
315+
We follow the [Conventional Commits](https://www.conventionalcommits.org/) specification for our commit messages and PR titles.
316+
317+
**PR Title Format:**
318+
```
319+
<type>[(scope)]: <description>
320+
```
321+
322+
**Types:** Defined in [.github/workflows/conventional-commit-check.yml](.github/workflows/conventional-commit-check.yml):
323+
* **feat**: New feature or functionality
324+
* **fix**: Bug fix
325+
* **docs**: Documentation only changes
326+
* **refactor**: Code refactoring without changing functionality
327+
* **perf**: Performance improvements
328+
* **test**: Adding or modifying tests
329+
* **build**: Build system or dependency changes
330+
* **ci**: CI/CD configuration changes
331+
* **chore**: Maintenance tasks
332+
* **revert**: Reverting a previous commit
333+
* **misc**: Miscellaneous changes
334+
335+
Note: Each PR/commit should have a single primary type. If your changes span multiple categories, choose the most significant one or consider splitting into separate PRs.
336+
337+
**Scope (optional):** The area of code affected. Common scopes include:
338+
339+
* `parser` - SQL parser and grammar
340+
* `analyzer` - Query analysis and validation
341+
* `planner` - Query planning, optimization, and rules (including CBO)
342+
* `spi` - Service Provider Interface changes
343+
* `scheduler` - Task scheduling and execution
344+
* `protocol` - Wire protocol and serialization
345+
* `connector` - Changes to connector functionality
346+
* `resource` - Resource management (memory manager, resource groups)
347+
* `security` - Authentication and authorization
348+
* `function` - Built-in functions and operators
349+
* `type` - Type system and type coercion
350+
* `expression` - Expression evaluation
351+
* `operator` - Query operators (join, aggregation, etc.)
352+
* `client` - Client libraries and protocols
353+
* `server` - Server configuration and management
354+
* `native` - Native execution engine
355+
* `testing` - Test framework and utilities
356+
* `docs` - Documentation
357+
* `build` - Build system and dependencies
358+
359+
Additionally, any connector name (e.g., `hive`, `iceberg`, `delta`, `kafka`) or plugin name (e.g., `session-property-manager`, `access-control`, `event-listener`) can be used as a scope.
360+
361+
**Description:**
362+
* Must start with a capital letter
363+
* Must not end with a period
364+
* Use imperative mood ("Add feature" not "Added feature")
365+
* Be concise but descriptive
366+
367+
**Breaking Changes:**
368+
* Use `!` after the type/scope (e.g., `feat!: Remove deprecated API`)
369+
* AND include `BREAKING CHANGE:` in the commit description footer with a detailed explanation of the change
370+
* Use to indicate any change that is not backward compatible as defined in the [Backward Compatibility Guidelines](presto-docs/src/main/sphinx/develop/release-process.rst#backward-compatibility-guidelines)
371+
372+
**Examples:**
373+
* `feat(connector): Add support for dynamic catalog registration`
374+
* `fix: Resolve memory leak in query executor`
375+
* `docs(api): Update REST API documentation`
376+
* `feat!: Remove deprecated configuration options` (breaking change)
377+
378+
### Single Commit PRs
379+
* **All PRs must be merged as a single commit** using GitHub's "Squash and merge" feature
380+
* The PR title will become the commit message, so it must follow the conventional commit format
381+
* Multiple commits within a PR are allowed during development for easier review, but they will be squashed on merge
382+
* If you need to reference other commits or PRs, include them in the PR description or commit body, not as separate commits
383+
384+
### Commit Message Guidelines
385+
* **PR Title/First Line**
386+
* Must follow conventional commit format
387+
* Limit to 50-72 characters when possible
388+
* If you cannot make it concise, you may be changing too much at once
389+
390+
* **PR Description/Commit Body**
391+
* Separate from title with a blank line
392+
* Wrap at 72 characters
393+
* Explain what and why, not how
394+
* Include:
395+
* Why the change is being made
396+
* What issue it addresses
397+
* Any side effects or limitations
398+
* Breaking changes or migration notes if applicable
399+
* Assume someone may need to revert your change during an emergency
400+
401+
* **Content Requirements**
402+
* All bug fixes and new features must have associated tests
403+
* Changes should be focused on a single topic
404+
* Code should pass all tests independently
405+
* Include documentation updates with code changes
349406
350407
* **Metadata**
351408
* If the commit was to solve a Github issue, refer to it at the end of a commit message in a rfc822 header line format.\
@@ -460,20 +517,13 @@ We use the [Fork and Pull model](https://docs.github.com/en/pull-requests/collab
460517
461518
- Make sure your code follows the [code style guidelines](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style), [development guidelines](https://github.com/prestodb/presto/wiki/Presto-Development-Guidelines#development) and [formatting guidelines](https://github.com/prestodb/presto/wiki/Presto-Development-Guidelines#formatting)
462519
463-
- Make sure you follow the [review and commit guidelines](https://github.com/prestodb/presto/wiki/Review-and-Commit-guidelines), in particular:
464-
465-
- Ensure that each commit is correct independently. Each commit should compile and pass tests.
466-
- When possible, reduce the size of the commit for ease of review. Consider breaking a large PR into multiple commits, with each one addressing a particular issue. For example, if you are introducing a new feature that requires certain refactor, making a separate refactor commit before the real change helps the reviewer to isolate the changes.
467-
- Do not send commits like addressing comments or fixing tests for previous commits in the same PR. Squash such commits to its corresponding base commit before the PR is rebased and merged.
468-
- Make sure commit messages [follow these guidelines](https://chris.beams.io/posts/git-commit/). In particular (from the guidelines):
520+
- Make sure you follow the [Commit Standards](#commit-standards) section above, which uses Conventional Commits format:
469521
470-
* Separate subject from body with a blank line
471-
* Limit the subject line to 50 characters
472-
* Capitalize the subject line
473-
* Do not end the subject line with a period
474-
* Use the imperative mood in the subject line
475-
* Wrap the body at 72 characters
476-
* Use the body to explain what and why vs. how
522+
- PR titles must follow the conventional commit format (e.g., `feat: Add new feature`, `fix: Resolve bug`)
523+
- All PRs will be squashed into a single commit on merge, so the PR title becomes the commit message
524+
- While developing, you can have multiple commits in your PR for easier review
525+
- Ensure each commit in your PR compiles and passes tests independently
526+
- The PR description should explain what and why, not how. Keep lines wrapped at 72 characters for better readability. Include context about why the change is needed, what issue it addresses, any side effects or breaking changes, and enough detail that someone could understand whether to revert it during an emergency.
477527
- Ensure all code is peer reviewed within your own organization or peers before submitting
478528
- Implement and address existing feedback before requesting further review
479529
- Make a good faith effort to locate example or referential code before requesting someone else direct you towards it

0 commit comments

Comments
 (0)