Skip to content

Feat: show commit msg length (closes #36)#40

Merged
JPeer264 merged 5 commits intomasterfrom
feat/commitMsgLength
Sep 6, 2017
Merged

Feat: show commit msg length (closes #36)#40
JPeer264 merged 5 commits intomasterfrom
feat/commitMsgLength

Conversation

@aichbauer
Copy link
Collaborator

@aichbauer aichbauer commented Sep 3, 2017

Maybe we should add character to the message? Like

The commit message is not allowed to be longer as 72 character, but is 85 character long. Consider writing a body.

Maybe we should also add the current characters to the warning if there are not enough characters. Like

The commit message has to be at least 10 character, but is only 8 character long.

@coveralls
Copy link

coveralls commented Sep 3, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 5f8cd43 on feat/commitMsgLength into 270d752 on master.

JPeer264
JPeer264 approved these changes Sep 4, 2017
View reviewed changes
Copy link
Owner

@JPeer264 JPeer264 left a comment

Choose a reason for hiding this comment

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

LGTM (just waiting for the AppVeyor CI)

@aichbauer
Copy link
Collaborator Author

aichbauer commented Sep 4, 2017

I had the same Problem in another repo.

Looks like its a bug from npm@5.4 only on windows.

Error: EPERM: operation not permitted on npm 5.4 on windows

Downgrading to npm@5.3 could help?

@aichbauer
Copy link
Collaborator Author

aichbauer commented Sep 4, 2017

So should we also add this to the The commit message has to be at least 10 character text? Should we add the word character or not?

@JPeer264
Copy link
Owner

JPeer264 commented Sep 4, 2017

This just occurs on npm@5+?

Yes, I guess adding the word characters may help some people understanding the warning.

@aichbauer
Copy link
Collaborator Author

aichbauer commented Sep 5, 2017 •
edited
Loading

It is a problem with npm@5.4 with npm@5.3 it should work... I faced the same problem with sgr... and it worked downgrading appveyor to npm@5.3... if no version is specified, appveyor installs the latest version... in this case npm@5.4...

should I downgrade the npm version in the appveyor.yml?

@coveralls
Copy link

coveralls commented Sep 5, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 9b158dd on feat/commitMsgLength into 270d752 on master.

@JPeer264
Copy link
Owner

JPeer264 commented Sep 6, 2017

@aichbauer ya please downgrade, if that is really the issue 👍

@coveralls
Copy link

coveralls commented Sep 6, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling d345b6f on feat/commitMsgLength into 270d752 on master.

@JPeer264 JPeer264 merged commit b28add9 into master Sep 6, 2017
@JPeer264 JPeer264 deleted the feat/commitMsgLength branch September 6, 2017 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JPeer264 JPeer264 JPeer264 approved these changes

@dani-sc dani-sc Awaiting requested review from dani-sc

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aichbauer @coveralls @JPeer264