ci: decrease the minBodyLength commit message limit to 20 chars by IgorMinar · Pull Request #37949 · angular/angular

IgorMinar · Jul 7, 2020

The motivation behind this change is to improve the productivity in the angular/angular repo
without sacrificing the original goal of having better understanding of changes within
the repo.

When the minBodyLength limit was originally introduced the goal was simple: force
committers to provide more contextual information for each change coming into the
repo. Too often we found ourselves in a situation where nobody understood what
motivated some of the changes and we needed more contextual info to know if the
change was correct, desirable, and still relevant (at a later point in time).

When the limit was introduced, we needed to pick a minimum body length - given no
data, and frustration with even big changes being committed with just a words in
the subject (along the lines of "fix(core): fixing a bug"), we overcompensated
and started off with a really high bar of minBodyLength set to 100 chars.

This turned out to be impractical and created a big friction point in making valid
changes in the angular/angular repo, and in fact caused some of the refactorings
and smaller changes to be either skipped or combined into other commits which
increased the burden for code reviewers.

The evidence in the friction points can be seen in the number of PRs that fail to pass
the current lint check on the first try, but more importantly also in the "creative"
writing that some of the committers are forced to resort to in order to satisfy the
current checks. Examples:

Given that we primarily care to document the motivation behind each change
(the answer to the ultimate question: WHY?), I've collected several common &
valid commit messages that are minimalistic and capture the WHY sufficiently:

Refactoring for readability.  => 28 chars
Improving variable naming.    => 26 chars
Additional test coverage.     => 25 chars
Cleaning up the code.         => 21 chars
Simplifies the code.          => 20 chars

These commit message bodies in addition to the commit message subject should
sufficiently satisfy the need to capture the context and motivation behind each
change without creating an undue burden on committers.

Example minimalistic commit message:

------

refactor(core): cleanup the expression parser

Simplifies the code.

----

Given this research, I'm decreasing the minBodyLenth in angular/angular to 20 chars.

The commit message quality can be additionally improved by implementing a commit message
template via .gitmessage that will guide the committers in following our commit message
guidelines via instructions provided in the form of in-the-flow help rather than as an after
the fact lint check.

More info: https://thoughtbot.com/blog/better-commit-messages-with-a-gitmessage-template

I'm intentionally deferring such change for a separate PR as not to complicate or delay the
minBodyLength limit decrease.

The motivation behind this change is to improve the productivity in the angular/angular repo without sacrificing the original goal of having better understanding of changes within the repo. When the minBodyLength limit was originally introduced the goal was simple: force committers to provide more contextual information for each change coming into the repo. Too often we found ourselves in a situation where nobody understood what motivated some of the changes and we needed more contextual info to know if the change was correct, desirable, and still relevant (at a later point in time). When the limit was introduced, we needed to pick a minimum body length - given no data, and frustration with even big changes being committed with just a words in the subject (along the lines of "fix(core): fixing a bug"), we overcompensated and started off with a really high bar of minBodyLength set to 100 chars. This turned out to be impractical and created a big friction point in making valid changes in the angular/angular repo, and in fact caused some of the refactorings and smaller changes to be either skipped or combined into other commits which increased the burden for code reviewers. The evidence in the friction points can be seen in the number of PRs that fail to pass the current lint check on the first try, but more importantly also in the "creative" writing that some of the committers are forced to resort to in order to satisfy the current checks. Examples: - angular@286fbf4 - angular@b2816a1 Given that we primarily care to document the motivation behind each change (the answer to the ultimate question: WHY?), I've collected several *common* & *valid* commit messages that are minimalistic and capture the WHY sufficiently: ``` Refactoring for readability. => 28 chars Improving variable naming. => 26 chars Additional test coverage. => 25 chars Cleaning up the code. => 21 chars Simplified the code. => 20 chars ``` These commit message bodies in addition to the commit message subject should sufficiently satisfy the need to capture the context and motivation behind each change without creating an undue burden on committers. Example minimalistic commit message: ------ refactor(core): cleanup the expression parser Simplifying the code. ---- Given this research, I'm decreasing the minBodyLenth in angular/angular to 20 chars. The commit message quality can be additionally improved by implementing a commit message template via `.gitmessage` that will guide the committers in following our commit message guidelines via instructions provided in the form of in-the-flow help rather than as an after the fact lint check. More info: https://thoughtbot.com/blog/better-commit-messages-with-a-gitmessage-template I'm intentionally deferring such change for a separate PR as not to complicate or delay the minBodyLength limit decrease.

pkozlowski-opensource

👍

petebacondarwin

I think at the end of the day, this is just a part of the reviewers' role - to ensure that the commit message actually adds value and provides context for future generations of maintainers.

I wonder how many people start their review of a PR by reading the commit message?

gkalpak

I love the idea of reducing the min body length 💯

I just wanted to point out that—according to the commit message guidelines—the commit message body should use imperative, present tense. So, Simplified the code. (20 chars) and Symplifying the code. (21 chars), which are shown as good examples, should be Simplify the code. (18 chars), which is less than 20 chars

On a more serious note, it might be a good idea to mention the body length requirement in the commit message guidelines (similar to how we mention the header length requirement). I wouldn't block this PR on this, though.

IgorMinar · Jul 7, 2020

@gkalpak you are right as always. 😄

jelbourn

LGTM

sonukapoor · Jul 7, 2020

@IgorMinar Thank you. I always had trouble coming up with 100 chars for the commit message when making minor fixes.

@gkalpak I can make the change in the commit guidelines.

gkalpak · Jul 8, 2020

Thx for offering, @sonukapoor. @IgorMinar has already taken care of it 😉

EDIT: I now realize the guidelines update is on a different PR, so putting it here for reference: #37951

IgorMinar · Jul 8, 2020

@sonukapoor thanks for offering, but as @gkalpak I updated the guidelines in #37951 - it's not ideal that the change is in a different PR but I didn't want to deal with merge conflicts between the two PRs.

The motivation behind this change is to improve the productivity in the angular/angular repo without sacrificing the original goal of having better understanding of changes within the repo. When the minBodyLength limit was originally introduced the goal was simple: force committers to provide more contextual information for each change coming into the repo. Too often we found ourselves in a situation where nobody understood what motivated some of the changes and we needed more contextual info to know if the change was correct, desirable, and still relevant (at a later point in time). When the limit was introduced, we needed to pick a minimum body length - given no data, and frustration with even big changes being committed with just a words in the subject (along the lines of "fix(core): fixing a bug"), we overcompensated and started off with a really high bar of minBodyLength set to 100 chars. This turned out to be impractical and created a big friction point in making valid changes in the angular/angular repo, and in fact caused some of the refactorings and smaller changes to be either skipped or combined into other commits which increased the burden for code reviewers. The evidence in the friction points can be seen in the number of PRs that fail to pass the current lint check on the first try, but more importantly also in the "creative" writing that some of the committers are forced to resort to in order to satisfy the current checks. Examples: - 286fbf4 - b2816a1 Given that we primarily care to document the motivation behind each change (the answer to the ultimate question: WHY?), I've collected several *common* & *valid* commit messages that are minimalistic and capture the WHY sufficiently: ``` Refactoring for readability. => 28 chars Improving variable naming. => 26 chars Additional test coverage. => 25 chars Cleaning up the code. => 21 chars Simplified the code. => 20 chars ``` These commit message bodies in addition to the commit message subject should sufficiently satisfy the need to capture the context and motivation behind each change without creating an undue burden on committers. Example minimalistic commit message: ------ refactor(core): cleanup the expression parser Simplifying the code. ---- Given this research, I'm decreasing the minBodyLenth in angular/angular to 20 chars. The commit message quality can be additionally improved by implementing a commit message template via `.gitmessage` that will guide the committers in following our commit message guidelines via instructions provided in the form of in-the-flow help rather than as an after the fact lint check. More info: https://thoughtbot.com/blog/better-commit-messages-with-a-gitmessage-template I'm intentionally deferring such change for a separate PR as not to complicate or delay the minBodyLength limit decrease. PR Close #37949

Git provides a way to create a commit message template via the `.gitmessage` file. Introduce an Angular-specific .gitmessage template based on the original Commit Message Guidelines. https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines If this template workflow is proven in practice, we can move the commit message guidelines into the .gitmessage file to prevent duplication of the content. This change is a follow up on angular#37949 and is inspired by info found in the following blog post: https://thoughtbot.com/blog/better-commit-messages-with-a-gitmessage-template

Git provides a way to create a commit message template via the `.gitmessage` file. Introduce an Angular-specific .gitmessage template based on the original Commit Message Guidelines. https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines If this template workflow is proven in practice, we can move the commit message guidelines into the .gitmessage file to prevent duplication of the content. This change is a follow up on #37949 and is inspired by info found in the following blog post: https://thoughtbot.com/blog/better-commit-messages-with-a-gitmessage-template PR Close #37951

angular-automatic-lock-bot · Aug 8, 2020

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

…lar#37949) The motivation behind this change is to improve the productivity in the angular/angular repo without sacrificing the original goal of having better understanding of changes within the repo. When the minBodyLength limit was originally introduced the goal was simple: force committers to provide more contextual information for each change coming into the repo. Too often we found ourselves in a situation where nobody understood what motivated some of the changes and we needed more contextual info to know if the change was correct, desirable, and still relevant (at a later point in time). When the limit was introduced, we needed to pick a minimum body length - given no data, and frustration with even big changes being committed with just a words in the subject (along the lines of "fix(core): fixing a bug"), we overcompensated and started off with a really high bar of minBodyLength set to 100 chars. This turned out to be impractical and created a big friction point in making valid changes in the angular/angular repo, and in fact caused some of the refactorings and smaller changes to be either skipped or combined into other commits which increased the burden for code reviewers. The evidence in the friction points can be seen in the number of PRs that fail to pass the current lint check on the first try, but more importantly also in the "creative" writing that some of the committers are forced to resort to in order to satisfy the current checks. Examples: - angular@286fbf4 - angular@b2816a1 Given that we primarily care to document the motivation behind each change (the answer to the ultimate question: WHY?), I've collected several *common* & *valid* commit messages that are minimalistic and capture the WHY sufficiently: ``` Refactoring for readability. => 28 chars Improving variable naming. => 26 chars Additional test coverage. => 25 chars Cleaning up the code. => 21 chars Simplified the code. => 20 chars ``` These commit message bodies in addition to the commit message subject should sufficiently satisfy the need to capture the context and motivation behind each change without creating an undue burden on committers. Example minimalistic commit message: ------ refactor(core): cleanup the expression parser Simplifying the code. ---- Given this research, I'm decreasing the minBodyLenth in angular/angular to 20 chars. The commit message quality can be additionally improved by implementing a commit message template via `.gitmessage` that will guide the committers in following our commit message guidelines via instructions provided in the form of in-the-flow help rather than as an after the fact lint check. More info: https://thoughtbot.com/blog/better-commit-messages-with-a-gitmessage-template I'm intentionally deferring such change for a separate PR as not to complicate or delay the minBodyLength limit decrease. PR Close angular#37949

googlebot added the cla: yes label Jul 7, 2020

IgorMinar requested a review from jelbourn July 7, 2020 00:09

IgorMinar force-pushed the ci/minBodyLenth-set-to-20 branch from ed8005e to 52df22a Compare July 7, 2020 00:09

IgorMinar added area: build & ci Related the build and CI infrastructure of the project action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jul 7, 2020

ngbot Bot modified the milestone: needsTriage Jul 7, 2020

pkozlowski-opensource approved these changes Jul 7, 2020

View reviewed changes

petebacondarwin approved these changes Jul 7, 2020

View reviewed changes

gkalpak approved these changes Jul 7, 2020

View reviewed changes

jelbourn approved these changes Jul 7, 2020

View reviewed changes

IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 7, 2020

atscott closed this in d0c0f25 Jul 8, 2020

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Aug 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: decrease the minBodyLength commit message limit to 20 chars - #37949

ci: decrease the minBodyLength commit message limit to 20 chars#37949
IgorMinar wants to merge 1 commit into
angular:masterangular/angular:masterfrom
IgorMinar:ci/minBodyLenth-set-to-20IgorMinar/angular:ci/minBodyLenth-set-to-20Copy head branch name to clipboard

IgorMinar commented Jul 7, 2020 •

edited

Loading

Uh oh!

pkozlowski-opensource left a comment

Uh oh!

petebacondarwin left a comment

Uh oh!

gkalpak left a comment

Uh oh!

IgorMinar commented Jul 7, 2020

Uh oh!

jelbourn left a comment

Uh oh!

sonukapoor commented Jul 7, 2020

Uh oh!

gkalpak commented Jul 8, 2020 •

edited

Loading

Uh oh!

IgorMinar commented Jul 8, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Aug 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

IgorMinar commented Jul 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

IgorMinar commented Jul 7, 2020

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

sonukapoor commented Jul 7, 2020

Uh oh!

gkalpak commented Jul 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IgorMinar commented Jul 8, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Aug 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

IgorMinar commented Jul 7, 2020 •

edited

Loading

gkalpak commented Jul 8, 2020 •

edited

Loading