Skip to content

feat(ci): Add scopes to conventional-commit-check.yml #26212

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 16, 2025
Merged

feat(ci): Add scopes to conventional-commit-check.yml #26212

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
24 changes: 24 additions & 0 deletions .github/workflows/conventional-commit-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ jobs:
revert
misc
requireScope: false
scopes: |
parser
analyzer
planner
spi
scheduler
protocol
connector
resource
security
function
type
expression
operator
client
server
native
Copy link
Contributor

@czentgr czentgr Oct 3, 2025

Choose a reason for hiding this comment

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

I wonder if we should have some native- subset of scopes.
Looks like you altogether changed your mind (compared to the original commit for the commit check).

Copy link
Contributor Author

@tdcmeehan tdcmeehan Oct 3, 2025

Choose a reason for hiding this comment

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

Yes, before I made it so you had to provide a scope, but I already see lots of made up and random scopes, so I think it would be beneficial to provide one if a scope is provided.

Perhaps native and native-pos? What other scopes might there be with a native prefix?

Copy link
Contributor

@czentgr czentgr Oct 21, 2025

Choose a reason for hiding this comment

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

Oops. What does native-pos mean?

Copy link
Contributor Author

@tdcmeehan tdcmeehan Oct 22, 2025

Choose a reason for hiding this comment

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

Presto on Spark

native-pos
testing
docs
build
ci
ui
plugin-[a-zA-Z][a-zA-Z0-9-]*
subjectPattern: ^[A-Z].*[^.]$
subjectPatternError: |
The PR title subject must start with a capital letter and not end with a period.
9 changes: 5 additions & 4 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,15 +334,15 @@ We follow the [Conventional Commits](https://www.conventionalcommits.org/) speci

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:
**Scope (optional):** The area of code affected. Valid scopes are defined in [.github/workflows/conventional-commit-check.yml](.github/workflows/conventional-commit-check.yml). 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
* `connector` - Changes to broader connector functionality or conector SPI
Copy link
Contributor

@czentgr czentgr Oct 21, 2025

Choose a reason for hiding this comment

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

Typo "conector".

Copy link
Contributor Author

@tdcmeehan tdcmeehan Oct 22, 2025

Choose a reason for hiding this comment

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

Raised a PR for that, thanks

* `resource` - Resource management (memory manager, resource groups)
* `security` - Authentication and authorization
* `function` - Built-in functions and operators
Expand All @@ -356,7 +356,7 @@ Note: Each PR/commit should have a single primary type. If your changes span mul
* `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.
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. These scopes should use the format `plugin-<name>` (e.g., `plugin-iceberg`, `plugin-password-authenticator`).

**Description:**
* Must start with a capital letter
Expand All @@ -370,10 +370,11 @@ Additionally, any connector name (e.g., `hive`, `iceberg`, `delta`, `kafka`) or
* 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`
* `feat(connector): Add support for dynamic catalog registration` (new feature for connectors)
* `fix: Resolve memory leak in query executor`
* `docs(api): Update REST API documentation`
* `feat!: Remove deprecated configuration options` (breaking change)
* `feat(plugin-iceberg): Add support for Iceberg table properties` (new feature in Iceberg, **NOTE: connectors are plugins**)

### Single Commit PRs
* **All PRs must be merged as a single commit** using GitHub's "Squash and merge" feature
Expand Down
Loading