Skip to content

chore(ci): Add action and documentation for semantic commits #26122

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.

Sign up for GitHub

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

Merged
tdcmeehan merged 1 commit into from
Oct 1, 2025
Merged

chore(ci): Add action and documentation for semantic commits #26122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .github/workflows/conventional-commit-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: Semantic Commit Check

on:
pull_request:
types:
- opened
- reopened
- edited
- synchronize

permissions:
contents: read

jobs:
semantic-pr:
concurrency:
group: ${{ github.workflow }}-semantic-commit-check-${{ github.event.pull_request.number }}
cancel-in-progress: true
name: Validate PR Title
runs-on: ubuntu-latest
steps:
- uses: amannn/action-semantic-pull-request@2d952a1bf90a6a7ab8f0293dc86f5fdf9acb1915 # v5.5.3
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
types: |
feat
fix
docs
refactor
perf
test
build
ci
chore
revert
misc
requireScope: false
subjectPattern: ^[A-Z].*[^.]$
Copy link
Contributor

@czentgr czentgr Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to also cover the list of scopes because we do define a list of acceptable ones?

Copy link
Contributor Author

@tdcmeehan tdcmeehan Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, also linked the CONTRIBUTING guide to here to save space and have one source of truth

Copy link
Contributor Author

@tdcmeehan tdcmeehan Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up reverting this, because we also need the ability to specify connectors and plugins as individual scopes. We'd have to hardcode them here, which seems less than ideal, so I chose to move them back to the documentation exclusively.

subjectPatternError: |
The PR title subject must start with a capital letter and not end with a period.
148 changes: 99 additions & 49 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,42 +310,99 @@ We recommend you use IntelliJ as your IDE. The code style template for the proje


## Commit Standards
* ### Commit Size
* Recommended lines of code should be no more than +1000 lines, and should focus on one single major topic.\
If your commit is more than 1000 lines, consider breaking it down into multiple commits, each handling a specific small topic.
* ### Commit Message Style
* **Separate subject from body with a blank line**
* **Subject**
* Limit the subject line to 10 words or 50 characters
* If you cannot make the subject short, you may be committing too many changes at once
* Capitalize the subject line
* Do not end the subject line with a period
* Use the imperative mood in the subject line
* **Body**
* Wrap the body at 72 characters
* Use the body to explain what and why versus how
* Use the indicative mood in the body\
For example, “If applied, this commit will ___________”
* Communicate only context (why) for the commit in the subject line
* Use the body for What and Why
* If your commit is complex or dense, share some of the How context
* Assume someone may need to revert your change during an emergency
* **Content**
* **Aim for smaller commits for easier review and simpler code maintenance**
* All bug fixes and new features must have associated tests
* Commits should pass tests on their own, not be dependent on other commits
* The following is recommended:
* Describe why a change is being made.
* How does it address the issue?
* What effects does the patch have?
* Do not assume the reviewer understands what the original problem was.
* Do not assume the code is self-evident or self-documenting.
* Read the commit message to see if it hints at improved code structure.
* The first commit line is the most important.
* Describe any limitations of the current code.
* Do not include patch set-specific comments.

Details for each point and good commit message examples can be found on https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages

### Conventional Commits
We follow the [Conventional Commits](https://www.conventionalcommits.org/) specification for our commit messages and PR titles.

**PR Title Format:**
```
<type>[(scope)]: <description>
```

**Types:** Defined in [.github/workflows/conventional-commit-check.yml](.github/workflows/conventional-commit-check.yml):
* **feat**: New feature or functionality
* **fix**: Bug fix
* **docs**: Documentation only changes
* **refactor**: Code refactoring without changing functionality
* **perf**: Performance improvements
* **test**: Adding or modifying tests
* **build**: Build system or dependency changes
* **ci**: CI/CD configuration changes
* **chore**: Maintenance tasks
* **revert**: Reverting a previous commit
Comment on lines 323 to 332
Copy link
Member

@imjalpreet imjalpreet Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use multiple types in the same PR/commit?

Copy link
Contributor Author

@tdcmeehan tdcmeehan Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the conventional commit spec, no, but the philosophy behind that is it should be a separate commit if you need multiple. In practice, this may be not be so strict, with for example some connector functionality being included with a change to the planner for example.

* **misc**: Miscellaneous changes

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.

**Scope (optional):** The area of code affected. Common scopes include:

* `parser` - SQL parser and grammar
* `analyzer` - Query analysis and validation
* `planner` - Query planning, optimization, and rules (including CBO)
* `spi` - Service Provider Interface changes
* `scheduler` - Task scheduling and execution
* `protocol` - Wire protocol and serialization
* `connector` - Changes to connector functionality
* `resource` - Resource management (memory manager, resource groups)
* `security` - Authentication and authorization
* `function` - Built-in functions and operators
* `type` - Type system and type coercion
* `expression` - Expression evaluation
* `operator` - Query operators (join, aggregation, etc.)
* `client` - Client libraries and protocols
* `server` - Server configuration and management
* `native` - Native execution engine
* `testing` - Test framework and utilities
* `docs` - Documentation
* `build` - Build system and dependencies

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.

**Description:**
* Must start with a capital letter
* Must not end with a period
* Use imperative mood ("Add feature" not "Added feature")
* Be concise but descriptive

**Breaking Changes:**
* Use `!` after the type/scope (e.g., `feat!: Remove deprecated API`)
* AND include `BREAKING CHANGE:` in the commit description footer with a detailed explanation of the change
* 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)

**Examples:**
* `feat(connector): Add support for dynamic catalog registration`
* `fix: Resolve memory leak in query executor`
* `docs(api): Update REST API documentation`
* `feat!: Remove deprecated configuration options` (breaking change)

### Single Commit PRs
* **All PRs must be merged as a single commit** using GitHub's "Squash and merge" feature
* The PR title will become the commit message, so it must follow the conventional commit format
* Multiple commits within a PR are allowed during development for easier review, but they will be squashed on merge
* If you need to reference other commits or PRs, include them in the PR description or commit body, not as separate commits

### Commit Message Guidelines
* **PR Title/First Line**
* Must follow conventional commit format
* Limit to 50-72 characters when possible
* If you cannot make it concise, you may be changing too much at once

* **PR Description/Commit Body**
* Separate from title with a blank line
* Wrap at 72 characters
* Explain what and why, not how
* Include:
* Why the change is being made
* What issue it addresses
* Any side effects or limitations
* Breaking changes or migration notes if applicable
* Assume someone may need to revert your change during an emergency

* **Content Requirements**
* All bug fixes and new features must have associated tests
* Changes should be focused on a single topic
* Code should pass all tests independently
* Include documentation updates with code changes

* **Metadata**
* 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.\
Expand Down Expand Up @@ -460,20 +517,13 @@ We use the [Fork and Pull model](https://docs.github.com/en/pull-requests/collab

- 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)

- Make sure you follow the [review and commit guidelines](https://github.com/prestodb/presto/wiki/Review-and-Commit-guidelines), in particular:

- Ensure that each commit is correct independently. Each commit should compile and pass tests.
- 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.
- 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.
- Make sure commit messages [follow these guidelines](https://chris.beams.io/posts/git-commit/). In particular (from the guidelines):
- Make sure you follow the [Commit Standards](#commit-standards) section above, which uses Conventional Commits format:

* Separate subject from body with a blank line
* Limit the subject line to 50 characters
* Capitalize the subject line
* Do not end the subject line with a period
* Use the imperative mood in the subject line
* Wrap the body at 72 characters
* Use the body to explain what and why vs. how
- PR titles must follow the conventional commit format (e.g., `feat: Add new feature`, `fix: Resolve bug`)
- All PRs will be squashed into a single commit on merge, so the PR title becomes the commit message
- While developing, you can have multiple commits in your PR for easier review
- Ensure each commit in your PR compiles and passes tests independently
- 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.
- Ensure all code is peer reviewed within your own organization or peers before submitting
- Implement and address existing feedback before requesting further review
- Make a good faith effort to locate example or referential code before requesting someone else direct you towards it
Expand Down
Loading