Skip to content
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

skip additional checks if a merge commit #1100

Merged
merged 11 commits into from
Jul 20, 2023

Conversation

delboy1978uk
Copy link
Contributor

Q A
Branch master for features and deprecations
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? no
Fixed tickets #1099

This PR makes Grump skip commit message checks (beyond the defaults) if it is a merge commit. This stops problems like JIRA ticket numbers not being in the main branches like master and develop.

Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

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

Looks like a good starting point. I've added some additional pointers.

src/Task/Git/CommitMessage.php Outdated Show resolved Hide resolved
src/Task/Git/CommitMessage.php Outdated Show resolved Hide resolved
Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

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

Looks good. Think this is about all the code it requires, nice! :)
Can you take a look at the comments I made and at the failing testsuite? Looks like a missing type in your function signature

src/Task/Git/CommitMessage.php Show resolved Hide resolved
src/Task/Git/CommitMessage.php Outdated Show resolved Hide resolved
@veewee veewee added this to the 2.1.0 milestone Jul 20, 2023
@veewee veewee merged commit bbb82b4 into phpro:v2.x Jul 20, 2023
@veewee
Copy link
Contributor

veewee commented Jul 20, 2023

Thanks for the addition

@delboy1978uk
Copy link
Contributor Author

@veewee Thanks so much for merging this feature! Can I also create a PR for v1.x ?

@veewee
Copy link
Contributor

veewee commented Jul 21, 2023

I wasn't really planning of porting new features back to v1 because of the maintenance overhead.

@delboy1978uk
Copy link
Contributor Author

@veewee Understandable, I'll use a fork of my own until the rest of the servers where I'm working are on PHP 8.1 (which shouldn't be long now). I'm looking forward to the release! Thanks again :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants