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.

Sign up for GitHub

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

document prompt for formatting good git-commit subjects. #232

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

document prompt for formatting good git-commit subjects. #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions code-style/git/.gitmessage
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#
# <footer>
#
# ** A good message should complete this sentence: when applied, this commit will... **
Copy link
Contributor

@iAmNathanJ iAmNathanJ May 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Should you replace the word "message" with "subject" (like you did in the README) to help clarify the recommendation is not for commit body/footer?

From a similar article:

Remember: Use of the imperative is important only in the subject line. You can relax this restriction when you’re writing the body.

https://chris.beams.io/posts/git-commit/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch!

Copy link

@anastasialanz anastasialanz Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We do mention in the README here that the body should use an imperative tone, but it seems like this Chris Beams guy says it doesn't need to be as strict. https://github.com/sparkbox/standard/blob/ab244d2331a527eb12dbd05febd30ddb1b0b1cfc/code-style/git/README.md#the-body

#
# ** TYPES **
# feat (new feature)
# fix (bug fix)
Expand Down
4 changes: 3 additions & 1 deletion code-style/git/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ Find the [allowed types above](#Allowed-Values-for-Types).

An [imperative tone][365] is also helpful in conveying what a commit does,
rather than what it did. For example, use **change**, not _changed_ or
_changes_.
_changes_. To put it another way: a good subject should complete this sentence:
["when applied, this commit will..."][drew-devault-link]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this type of prompt is really helpful. I'm trying to put myself in the committer's shoes and see how this changes my instincts.

when applied, this commit will...

  • Adds dependabot config
  • adds XYZController

I'm not sure this commit will puts my focus on what the commit changes.

If we switched the wording a little to when this commit is applied, ____

  • Dependabot is run every day
  • XYZController handles /xyz/
  • Users can see their XYZs

It's not perfect either, but I felt like it took the focus away from what the commit does, which is clear in the diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like either prompt.
Is the issue that this prompt necessitates filling out the body in a useful way? Do we need to add a prompt for the body too?

Copy link

@anastasialanz anastasialanz Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I've been trying out the above when this commit is applied, ___ to help me write better commit subjects recently. I think using this commit will __ makes you use a verb to start the subject like "add", "fix", "remove" which I can see in @cromwellryan's opinion could state the obvious if you looked at the code to see what was added or removed. Using when this commit is applied can help you focus more on the impact of removing or adding code. For instance, now that we added a Dependabot config, the impact is that Dependabot will run every week. I'm sure there are times when using a verb like "add" or "remove" would be sufficient in some cases too like when we remove peoples ssh keys from a repo.

All this is to stay that I think I like when this commit is applied, ____ a little bit better.

I don't think we need a prompt for the body. We already mention that it should explain the "why".

From the referenced article:
https://chris.beams.io/posts/git-commit/

Use the body to explain what and why vs. how


### Funtip

Expand Down Expand Up @@ -327,3 +328,4 @@ Voila! You're done. You've successfully rebased a branch onto the master branch!
[example PR template]: ./PULL_REQUEST_TEMPLATE.md
[example issue template]: ./ISSUE_TEMPLATE.md
[Create a branch]: #naming-branches
[drew-devault-link]: https://drewdevault.com/2019/02/25/Using-git-with-discipline.html