ezard/semantic-prs

A GitHub app to check that pull requests follow the Conventional Commits spec

semantic-prs's Issues

Make it opt-in as original, not opt-out

Please leave it up to each repository to enable it explicitly. 99% of orgs/users have mixed content, and forcing users to disable it other repositories, instead of enabling it where it is needed, is bad UX in my opinion. Using app installation to do that is also something I would not recommend.

I did not test it, but from the look of the code, if you want to have org defaults, you can create special repo .github and octokit-plugin-config will try to load required file from that repo as a fallback. If course, that can be also used to disable it globally, but I would still not recommend going this route.

Of course, its your app and configure it however you see fit, but I'll personally never use a tool, that forces itself into my way every time, instead of serving me when I need it.

Plugin does not recognise dependabots Build as a valid pr title

Describe the bug

dependabot sometimes creates pr that start with "Build" which is valid according to conventional commit spec but semantic pr always fails for those prs

semantic.yml contents

Sample PR Title(s) (if relevant)

Build(deps-dev): Bump typescript from 5.0.4 to 5.1.3

Sample Commit Message(s) (if relevant)

No response

Provide more helpful error message

Status Quo

currently, failure message does not describe why the check failed.

semantic-prs/functions/src/handle-pull-request-change.ts

Lines 90 to 102 in afe753c

	function getSemanticState(): SemanticState {
	if (!config.enabled) {
	return new InfoSemanticState(true, 'skipped; check enabled in semantic.yml config');
	} else if (config.titleOnly) {
	return new SuccessFailureSemanticState(hasSemanticTitle, 'ready to be squashed', 'add a semantic PR title');
	} else if (config.commitsOnly) {
	if (config.anyCommit) {
	return new SuccessFailureSemanticState(
	someCommitsSemantic,
	'ready to be merged or rebased',
	'add a semantic commit',
	);
	} else {

The problem

while using semantic PR in https://github.com/cataclysmbnteam/Cataclysm-BN, we found out that new contributors had a hard time figuring out why their PR failed. for example:

I have no clue what the Semantic PR bot is angry about with my latest PR's title, but oh well. At least all the actually important tests came back okay lol

giving context-aware error message will help new contributors greatly.

The Proposal

make error message aware of configuration context. for example, with this configuration, each error cases could have been:

Malformed syntax

 feat : foo 

• hint: remove invalid leading space before type feat
• hint: remove invalid space between type feat and :
• hint: remove invalid trailing space after description foo

-  feat : foo 
+ feat: foo

Invalid type

feature: foo

• invalid type feature.
• accepted types are: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert.
• hint: did you mean feat?

- feature: foo
+ feat: foo

Invalid Scope

feat(): bar

• empty scopes are not allowed.
• hint: remove ()

- feat(): bar
+ feat: bar

feat(ui): baz

• invalid scope ui.
• accepted scopes are: content, UI, i18n, balance, ...
• hint: did you mean UI?

- feat(ui): baz
+ feat(UI): baz

Additional Context

• I'm interested in opening a PR.
• If errors are too long to fit in, adding error comment might be better.

Details report the violations

Currently, if semantic-prs reports fails, the "details" button jump to this repo instead of a concrete info about violations.

Hacked

regular expression support

hi, i'd like to apply conventional PR titles for our project, and thought it'd be neat to define valid scopes by regex. for example:

scopes:
  - 'mods/(\w+)'

would match titles such as

feat(mods/foo): ...
feat(mods/bar): ...

i'm interested in opening a PR for this feature.

relevent lines:

semantic-prs/functions/src/is-message-semantic.ts

Lines 36 to 37 in 2915f0c

	const isScopeValid = !scopes || !scope || scope.split(',').every(scope => scopes.includes(scope));
	const isTypeValid = (types.length > 0 ? types : commitTypes).includes(type);

Stacy Sager End point hacked

semantic.yaml not being read?

Describe the bug

I've set targetUrl to a custom URL, but as the screenshot shows below, it's not being used:

The semantic.yaml file is already merged to main and the branch PR is up to date with main.

In fact, I'm not even sure it's using the semantic.yaml config at all, as I've set anyCommit: false, and titleAndCommits: true and it's only checking for commit message instead of both.

The file is located at .github/semantic.yaml

semantic.yml contents

enabled: true

titleOnly: false

commitsOnly: false

titleAndCommits: true

anyCommit: false

types:
  - feat
  - fix
  - docs
  - refactor
  - ci
  - chore

allowMergeCommits: true

allowRevertCommits: false

targetUrl: https://www.conventionalcommits.org/en/v1.0.0/

Sample PR Title(s) (if relevant)

test

Sample Commit Message(s) (if relevant)

feat: test

support spaces in scopes

Current Situation

feat(foo, bar): baz -> Fails
feat(foo,bar): baz -> OK

Expected Situation

feat(foo, bar): baz -> OK
feat(foo,bar): baz -> OK

I'm willing to open a PR.

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): update dependency eslint to v9
• chore(deps): update dependency firebase-functions to v5
• chore(deps): update dependency probot to v13
• chore(deps): update node.js to v20 (node, @types/node)
• chore(deps): update typescript-eslint monorepo to v8 (major) (@typescript-eslint/eslint-plugin, @typescript-eslint/parser)
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

github-actions

.github/workflows/cache-dependencies.yml

• actions/checkout v4
• actions/setup-node v4
• actions/cache v4

.github/workflows/ci.yml

• actions/checkout v4
• actions/setup-node v4
• actions/cache v4

npm

functions/package.json

• @conventional-commits/parser 0.4.1
• conventional-commit-types 3.0.0
• firebase-functions 4.9.0
• probot 12.4.0
• @octokit/webhooks-types 7.5.1
• @stryker-mutator/core 8.5.0
• @stryker-mutator/jest-runner 8.5.0
• @stryker-mutator/typescript-checker 8.5.0
• @types/jest 29.5.12
• @types/node 18.19.47
• @typescript-eslint/eslint-plugin 7.18.0
• @typescript-eslint/parser 7.18.0
• eslint 8.57.0
• eslint-config-prettier 9.1.0
• eslint-plugin-import 2.29.1
• firebase-functions-test 3.3.0
• firebase-tools 13.16.0
• jest 29.7.0
• nock 13.5.5
• prettier 3.3.3
• smee-client 2.0.3
• ts-jest 29.2.5
• typescript 5.5.4
• node 18

---

• Check this box to trigger a request for Renovate to run again on this repository

Semantic PR check is false even there are commits started with "feat: "

Describe the bug

I have a PR (pawellabaj/auto-record#102) with many commits with message: "feat: Allowed to implement extension for compact constructor updating", eg. pawellabaj/auto-record@c3a6c50

Semantic PR is false

semantic.yml contents

n/a

Sample PR Title(s) (if relevant)

No response

Sample Commit Message(s) (if relevant)

feat: Allowed to implement extension for compact constructor updating

Enable Semantic PRs doesn't apply for existing PRs

After enable Semantic PRs on bytebase/bytebase and close & reopen PR bytebase/bytebase#1445, it seems Semantic PRs app doesn't report the status. Only new (created) PRs will trigger the App.

Is it possible to (re)check and report the status when a PR is reopened?

missing space in PR title does not get caught

Describe the bug

from cataclysmbnteam/Cataclysm-BN#4329

semantic.yml contents

enabled: true
titleOnly: true
targetUrl: 'https://docs.cataclysmbn.org/en/contribute/changelog_guidelines/'
types:
  - feat
  - fix
  - docs
  - style
  - refactor
  - perf
  - test
  - build
  - ci
  - chore
  - revert

# irrelevant configs skipped   

https://github.com/cataclysmbnteam/Cataclysm-BN/blob/59eb28f4a7404d1ec2f4be1eaa387d4317a8a812/.github/semantic.yml

Sample PR Title(s) (if relevant)

feat(content):Cooking with insect eggs

Sample Commit Message(s) (if relevant)

No response

Ability to use a common org-wide yaml config?

Hello! We've been using this app happily for quite a while now and we want to expand to have it enabled on all repos in our org.

Would it be possible for the GitHub App to look for org-wide config defaults in a specific repo within the org?

So the decision tree would then be:

1. If the repo has a semantic.yml file, use it.
2. Else if the org has a centrally available semantic.yml file, use it.
3. Else, use the GitHub App defaults.

Docs don't reflect that `semantic.yml` is read from default branch

The README says that the app can be configured by:

creating a semantic.yml file in your .github directory...

However, if you create a PR adding this semantic.yml file, the settings will not be reflected in that PR. Even a PR into your new branch with a semantic.yml will not reflect the config changes. It looks like Probot's context.config("semantic.yml") function gets the config file from the repositories default branch. This means a user's config settings won't be reflected in PRs until the semantic.yml is merged into the default branch.

My all commits are following conventional commit format but the `Semantic PRS` github app is showing error

I think that this problem is due to scopes when I don't use scopes than it is working fine

Private repositories

Hey, thanks for this drop-in replacement for Semantic Pull Requests!

The only issue we have is that the check doesn't appear to run in our private repositories, I'm guessing this isn't intentional behaviour?

Thanks

[email protected]

Merge queues are not supported

Describe the bug

When adding a PR to a merge queue in a repo where this check is required according to branch protection rules, it hangs with "Waiting for status to be reported".

I think the fix could be as simple as triggering this check on a pull_request.enqueued event.

Thanks!

semantic.yml contents

# Config ref: https://github.com/Ezard/semantic-prs

# Validate the PR title, and ignore all commit messages
titleOnly: true

# Provides a custom URL for the "Details" link, which appears next to the success/failure message from the app:
targetUrl: https://docs.meltano.com/contribute/merge#semantic-prs

# The values allowed for the "type" part of the PR title/commit message.
# e.g. for a PR title/commit message of "feat: add some stuff", the type would be "feat"
types:
  - ci
  - chore
  - build
  - break
  - change
  - docs
  - feat
  - fix
  - perf
  - refactor
  - revert
  - style
  - test

# The values allowed for the "scope" part of the PR title/commit message.
# e.g. for a PR title/commit message of "feat(awesome-feature): add some stuff",
#      the scope would be "awesome-feature"
scopes:
  - core
  - cli
  - cloud
  - deps
  - deps-dev

Sample PR Title(s) (if relevant)

No response

Sample Commit Message(s) (if relevant)

No response

Custom readme ref on failure

I am very much enjoying this app and we're implementing here in our open source project: meltano/sdk#686

We added a section to our contributors guide with description of expectations. I'd like to request (and perhaps contribute if helpful) a feature to customize the "more details" error url to accept a custom link.

Is that something that would be accepted? And if yes, could someone kindly point me in the right direction?