Remove period from "Update draft release notes..." commit message #622
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The first line of a commit message should not have a period (`.`) I am not aware of any official Git documentation for the statement above. However, it has been a convention in all the projects and companies I worked on. The style is also present in the example commit message from the official Git book. See https://git-scm.com/docs/MyFirstContribution#:~:text=You%20will%20be%20presented%20with%20your,A%20U%20Thor%20%3Cauthor%40example.com%3E The style is also used in other sources, such as: - https://commit.style/ - https://www.gitkraken.com/learn/git/best-practices/git-commit-message#using-imperative-verb-form - https://thoughtbot.com/blog/5-useful-tips-for-a-better-commit-message One could argue that the 50 to 72 character length and the period omission were characters economy constraints from back in the day of small screens and text-only interfaces. While that is true, I find the are many benefits in forcing oneself to be concise in the commit title/summary. As for short line length, it works well for people like me, who use big font sizes because their eyes strain otherwise. But aside from all the justifications above, having commit message titles without a period is the existing convention in this repo, which make this change a no brainer.
mokagio
commented
Dec 3, 2024
| @@ -41,7 +41,7 @@ def self.extract_notes(release_notes_file_path, version) | |||
|
|
|||
| def self.check_and_commit_extracted_notes_file(file_path, version) | |||
| Action.sh("git add #{file_path}") | |||
| Action.sh("git diff-index --quiet HEAD || git commit -m \"Update draft release notes for #{version}.\"") | |||
| Action.sh("git diff-index --quiet HEAD || git commit -m \"Update draft release notes for #{version}\"") | |||
There was a problem hiding this comment.
Aside from the formatting, it would have been good to update the code to use either the Fastlane Git API or the git wrapper gem that I think we already have setup.
But I'm in the middle of my support rotation and already feel guilty for indulging in this fix.
There was a problem hiding this comment.
Took the liberty to do exactly that in 72b0592.
Tested by running extract_release_notes_for_version in a dummy branch of PCiOS, worked as expected (first run created the commit with changes to the release notes files, second call detected there was nothing new to commit)
AliSoftware
approved these changes
Dec 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does it do?
The first line of a commit message should not have a period (
.)I noticed this while looking at Automattic/pocket-casts-ios#2484 in the context of p1733261974076509-slack-C029BMLUGRX.
I am not aware of any official Git documentation for the statement above. However, it has been a convention in all the projects and companies I worked on.
The style is also present in the example commit message from the official Git book. See
https://git-scm.com/docs/MyFirstContribution#:~:text=You%20will%20be%20presented%20with%20your,A%20U%20Thor%20%3Cauthor%40example.com%3E
The style is also used in other sources, such as:
One could argue that the 50 to 72 character length and the period omission were characters economy constraints from back in the day of small screens and text-only interfaces. While that is true, I find the are many benefits in forcing oneself to be concise in the commit title/summary.
As for short line length, it works well for people like me, who use big font sizes because their eyes strain otherwise. 🤓
But aside from all the justifications above, having commit message titles without a period is the existing convention in this repo, which make this change a no-brainer.
Checklist before requesting a review
bundle exec rubocopto test for code style violations and recommendationsspecs/*_spec.rb) if applicablebundle exec rspecto run the whole test suite and ensure all your tests passCHANGELOG.mdfile to describe your changes under the appropriate existing###subsection of the existing## Trunksection.MIGRATION.mdfile to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.