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: allow TODO in comments #9

Merged
merged 1 commit into from
Oct 29, 2019
Merged

fix: allow TODO in comments #9

merged 1 commit into from
Oct 29, 2019

Conversation

blacha
Copy link
Member

@blacha blacha commented Oct 24, 2019

This allows TODO: to be present in comments, it is useful to explain to future developers areas of concern inside of a codebase.

@blacha
Copy link
Member Author

blacha commented Oct 24, 2019

@dwsilk 80 character limit on git commit body messages! that is tiny, is this something we could configure?

@blacha
Copy link
Member Author

blacha commented Oct 24, 2019

Looking at https://www.conventionalcommits.org/en/v1.0.0/

A longer commit body MAY be provided after the short description, providing additional contextual information about the code changes. The body MUST begin one blank line after the description.
A commit body is free-form and MAY consist of any number of newline separated paragraphs.

which implies this 80 character limit is not really part of the spec

@dwsilk
Copy link
Member

dwsilk commented Oct 24, 2019

It isn't a character limit, it's a line length limit - it wants you to wrap text at character 80. It's not part of conventional commits, it's part of the standard rules in gitlint.

We could configure it to be longer by default in .gitlint, and then the configuration is also more visible, e.g.

[body-max-line-length]
line-length=120

This is the link @palmerj provided on the original pull request on commit linting, it recommends wrapping text at 72 chars: https://chris.beams.io/posts/git-commit/#wrap-72

On this particular issue, I feel like there are two different groups of users and no ideal middle ground that suits both.

@palmerj
Copy link

palmerj commented Oct 24, 2019

This is the link @palmerj provided on the original pull request on commit linting, it recommends wrapping text at 72 chars: https://chris.beams.io/posts/git-commit/#wrap-72

I think it would be good to have summary comment at 50 characters. Also having the body context wrapped (i.e newline) at 72 characters is the nicest for me when viewing git log messages on the terminal

@dwsilk
Copy link
Member

dwsilk commented Oct 28, 2019

Created an issue regarding this here: #12

The discussion will be lost on this unrelated PR.

@dwsilk
Copy link
Member

dwsilk commented Oct 28, 2019

@blacha could you change your commit body to:

TODO is useful to explain areas of concern to future developers.

to get this through before we resolve the commit message body length configuration.

TODO is useful to explain areas of concern to future developers.
@dwsilk dwsilk merged commit aba2012 into master Oct 29, 2019
@dwsilk dwsilk deleted the bug/allow-todo branch October 29, 2019 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants