Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

commit messages break conventional commit rules #292

Closed
ianwremmel opened this issue Dec 8, 2018 · 18 comments
Closed

commit messages break conventional commit rules #292

ianwremmel opened this issue Dec 8, 2018 · 18 comments

Comments

@ianwremmel
Copy link

It looks like sometime about two weeks ago, build became Build which violates conventional-commits.

@greysteil
Copy link
Contributor

Can you ping me the repo on which that happened? We now have a bunch of users who want capitalisation, but should be automatically detecting your preference.

@ianwremmel
Copy link
Author

The repo in question was https://github.com/ianwremmel/relayhook (recently renamed from https://github.com/ianwremmel/displace).

I didn't realize I can configure the messages, so happy to do that if that's the best solution.

@greysteil
Copy link
Contributor

I am at a total loss here. I've re-run the logic locally and it doesn't capitalise, and reading through I can't see any way how it accidentally would. The logic in question is here. What's incredible to me is that one of the PRs that I can see Dependabot got this wrong on was created today at 6am, so would have been running that exact logic, but it doesn't reproduce for me locally.

Don't suppose there's any chance you've recently amended the commits on your master branch or anything like that?

On the plus side there's an easy fix - if you use GitHub's "squash and merge" option when you merge the two open PRs you can change the subject line of the commits from there. Dependabot will then pick up that those commits use lowercase, and use that going forward.

Sorry about this one!

@ianwremmel
Copy link
Author

no worries, thanks for looking into it!

It looks like two of my repos have had this happening for about week. That logic looks sound, so it didn't make sense, but I've now looked at my commit history and I have a theory.

GitHub merge commits are of the form Merge pull request #496 from ianwremmel/tf-h. If they get included in the set of semantic_messages, that may drag down the average enough to trigger capitalization logic.

@greysteil
Copy link
Contributor

I thought it could be that, but Dependabot is smart about what commits it looks at when considering whether semantic commits are being capitalised. I just can't reproduce, which is maddening! I'll take another look in the morning and see if I can figure anything out.

@greysteil
Copy link
Contributor

I'm going to close this because I can't reproduce it and don't have any further way to debug, but if you see this again please comment and I'll re-open!

@ianwremmel
Copy link
Author

Sounds good, will do. Thanks for looking into it!

@exactlyaron
Copy link

Hi @greysteil I have just realised this is happening too.

This one is "build" - wmfs/pg-model#81

Whereas this one is "Build" - wmfs/pg-model#82

Or you can see some here which I haven't merged in yet or any of our other repos actually.

https://github.com/wmfs/tymly-solr-plugin/pulls

@exactlyaron
Copy link

Just thinking back the only thing I have done recently to the config of dependabot, a couple of weeks a go I started adding our scoped @wmfs packages to the whitelist on dependabot to be merged automatically.

@greysteil
Copy link
Contributor

Thanks @exactlyaron. I’m pretty sure this is from an old version of Dependabot. If you previously merged a PR with the capital then future PRs would be picking that up and using it.

The fix is to amend the commit message on a Dependabot PR to use lowercase when merging it - Dependabot will use the latest commit message when determining what to use for future PRs.

@exactlyaron
Copy link

Thanks @greysteil will that need to be done across all repos?

@greysteil
Copy link
Contributor

Anywhere where you’re getting the bad capitalisation. Sorry! 😞

@exactlyaron
Copy link

Okay no worries. Better get opening some tabs! 😆

@ianwremmel
Copy link
Author

@greysteil it happened again. all of the dbot PRs got capitalized. I think it's because dbot saw this GitHub merge commit.

@greysteil
Copy link
Contributor

Just looking through those PRs - as far as I can tell this still dates from 7th December and this pull request that was merged. Am I missing something?

@ianwremmel
Copy link
Author

Oh, interesting. I hadn't realized that project had run into the issue before. I think I added commitlint recently. so, dbot only looks at its own commits for capitalization now? There are a lot of recent lowercase commits that got opened before that page of PRs

@greysteil
Copy link
Contributor

Has been looking for the past 4 months (since this issue, and the comments on it). Fix is still the same - if you amend the commit message on a Dependabot PR to use lowercase when merging it - Dependabot will use the latest commit message when determining what to use for future PRs.

@ianwremmel
Copy link
Author

Got it, thanks!

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

No branches or pull requests

3 participants