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

fix: examples should follow Conventional Commits conventions #259

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tkrotoff
Copy link

@tkrotoff tkrotoff commented Apr 3, 2024

Conventions Commits and Angular conventions don't use a capital letter after colon: none of their examples use a capital letter after colon.

action-semantic-pull-request examples should not deviate from the conventions it is supposed to enforce.


What about English conventions? https://en.wikipedia.org/wiki/Colon_(punctuation)#Use_of_capitals

While it is acceptable to capitalise the first letter after the colon in American English, it is not the case in British English, except where a proper noun immediately follows a colon.

In British English, and in most Commonwealth countries, the word following the colon is in lower case unless it is normally capitalized for some other reason, as with proper nouns and acronyms.

American English permits writers to similarly capitalize the first word of any independent clause following a colon. This follows the guidelines of some modern American style guides, including those published by the Associated Press and the Modern Language Association. The Chicago Manual of Style, however, requires capitalization only when the colon introduces a direct quotation, a direct question, or two or more complete sentences.

In many European languages, the colon is usually followed by a lower-case letter unless the upper case is required for other reasons, as with British English.

tkrotoff added 2 commits April 3, 2024 11:25
Also in English, most of the time you don't write a capital letter after colon
@tkrotoff tkrotoff force-pushed the no-capital-letter-after-colon branch from 3183a7b to dc9ab8f Compare April 3, 2024 09:49
function printAvailableTypes() {
return `Available types:\n${types
.map((type) => {
let bullet = ` - ${type}`;

if (types === defaultTypes) {
bullet += `: ${conventionalCommitTypes.types[type].description}`;
bullet += `: ${removeCapitalLetter(conventionalCommitTypes.types[type].description)}`;
Copy link
Author

@tkrotoff tkrotoff Apr 3, 2024

Choose a reason for hiding this comment

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

Before:

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

After:

Available types:
 - feat: a new feature
 - fix: a bug fix
 - docs: documentation only changes
 - style: changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: a code change that neither fixes a bug nor adds a feature
 - perf: a code change that improves performance
 - test: adding missing tests or correcting existing tests
 - build: changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: other changes that don't modify src or test files
 - revert: reverts a previous commit

Copy link
Owner

@amannn amannn left a comment

Choose a reason for hiding this comment

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

Thanks for starting this discussion! I see your point and I'm aware that the examples from Conventional Commits don't use uppercase descriptions.

However, the only part of the spec in regard to descriptions is this part:

A description MUST immediately follow the colon and space after the type/scope prefix. The description is a short summary of the code changes, e.g., fix: array parsing issue when multiple spaces were contained in string.

So strictly speaking, the spec doesn't mention a necessity for the casing that follows the colon.

The reason why I advocate for uppercase descriptions, is because pragmatically the places where a description shows up, it's often presented as a sentence.

E.g. from the changelog of this lib:

Features
Use github.api_url as default for githubBaseUrl (#243 by @fty4) (4d5734a)

I'd say the formatting would be off in these places if the description would be lowercase.

That being said, I understand that people have different opinions. To be honest, I don't know the preference of the whole user base of this GitHub Action, but I think it's fair to assume that people have different opinions—so it's hard to pick one that is universally true.

As for the implementation and docs, we demonstrate an uppercase description, since also semantic-release is mentioned as a primary tool to be used in conjunction with this action. If someone doesn't like this, you can always enforce lowercase descriptions with a regex or use custom types, which will avoid printing the examples you've changed in this PR.

My standpoint is that this library adheres to Conventional Commits, but for areas that are not specified, we'll use pragmatic judgment for what provides a good end result.

Hope this helps!

@tkrotoff
Copy link
Author

tkrotoff commented Apr 3, 2024

From the spec:

Are the types [e.g. fix, feat, chore...] in the commit title uppercase or lowercase?
Any casing may be used, but it’s best to be consistent.

Kind of strange all examples use lowercase and they don't talk about the case for description.
It's like a non-assumed deliberate choice.

Related issues:


From what I've seen (in general, not specifically related to this GitHub Action users), most (90%?) use lowercase after colon (e.g feat: hello and not feat: Hello).


the places where a description shows up, it's often presented as a sentence
E.g. from the changelog of this lib

I see, it's a valid point!

"Official" answer: conventional-commits/conventionalcommits.org#83 (comment)

the changelogs tools may do it on the fly while generating it



What we do in our team:

  • Git commits (devs audience and not good granularity for the changelog anyway): regular English sentence (so no Conventional Commits)
  • PR titles (proper granularity for the changelog): Conventional Commits enforced using action-semantic-pull-request
  • Changelog (users audience): generated using GitHub "Draft a new release" > "Generate release notes"
    • GitHub simply lists the PR titles (e.g. * feat: hello), we don't alter what GitHub generates (it's good enough for us), we don't regroup entries by categories like you do

If you want better changelog formatting, Changesets is a probably a better option.

@foundatron
Copy link

foundatron commented Nov 15, 2024

Hi, I came here to bikeshed about casing. The internet is great. I recently tried this GH action (loved it) and then noticed it didn't pick up my casing error (perhaps an error in the eye of the beholder). I read through through this thread and those conventional commit docs, and was like yes, @amannn is correct, it's not in the spec. To which I say, the spec is wrong! It should all be lowercase!!!

And commitlint's config-conventional plugin agrees with me

echo "feat: Big titles" | npx commitlint
⧗   input: feat: Big titles
✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

Anyways, @amannn, do what you want to do, I'm just amused to add to this thread. Thanks for the GH Action.

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.

3 participants