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

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

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

Conversation

asimpson
Copy link
Contributor

No description provided.

@@ -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

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

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

@@ -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

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

@bengoshow
Copy link

This PR is a couple months old now, but as I'm getting up to speed on "the Sparkbox way" to do git, are there any new thoughts on this? @jordanleven

@asimpson
Copy link
Contributor Author

@bengoshow not that I'm aware of. The general advice of this PR is valid, the semantics are what we're discussing, so if the advice is useful great! :)

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

Successfully merging this pull request may close these issues.

5 participants