-
Notifications
You must be signed in to change notification settings - Fork 2.2k
docs: Address PR titles in CONTRIBUTING.md #3952
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
Conversation
Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
|
cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3952 +/- ##
=======================================
Coverage 92.45% 92.45%
=======================================
Files 203 203
Lines 14980 14980
=======================================
Hits 13850 13850
Misses 927 927
Partials 203 203 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CONTRIBUTING.md
Outdated
| 5. You may optionally prefix the PR title with the type of PR it is, in lower case, | ||
| followed by a colon. For example, `feat:`, `chore:`, `fix:`, `docs:`, etc. | ||
| For breaking API changes, add an exclamation point. | ||
| For example, `feat!:`, `chore!:`, `fix!:`, `docs!:`, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to have the docs!: commit? ! means breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's briefly describe also about the !.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah! Good call. I'm on my phone right now, so if you feel like adding suggestions that I can commit, great... otherwise, I will address your review requests later when I get back to my laptop. 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries at all, this can absolutely wait. 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's briefly describe also about the
!.
So for this comment, I'm not sure what else to add since I said this means a "breaking API change".
Ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to go ahead and merge this, then more improvements can come in separate PRs.
CONTRIBUTING.md
Outdated
| * When possible, try to make smaller, focused PRs (which are easier to review | ||
| and easier for others to understand). | ||
|
|
||
| ### Use proper commit messages and PR titles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have similar info in this document: https://github.com/google/go-github/blob/master/CONTRIBUTING.md#submitting-a-patch
Maybe we should edit it instead.
stevehipwell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
|
Thank you, @alexandear and @stevehipwell! |
No description provided.