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

feat(config-conventional): footer/body-max-line #436

Merged

Conversation

@Fetz
Copy link
Contributor

Fetz commented Sep 9, 2018

Description

  • Updated the config-conventional with footer/body max length
  • Added tests to all the rules of config-conventional
  • Updated the documentation of config-conventional to include missing rules

Motivation and Context

Related to: https://github.com/marionebl/commitlint/issues/303#issuecomment-418525320

Usage examples

# footer-line-max-length
echo "fix: some message
 BREAKING CHANGE: footer with multiple lines
has a message that is way too long and will break the line rule 'line-max-length' by several characters" # fails

 echo "fix: some message
 BREAKING CHANGE: footer with multiple lines
but still no line is too long" # passes
# body-line-max-length
echo "fix: some message
 body with multiple lines
has a message that is way too long and will break the line rule 'line-max-length' by several characters" # fails

echo "fix: some message
 body with multiple lines
but still no line is too long" # passes

How Has This Been Tested?

  • Added tests
  • Found a bug with the max-line-length when the commit message includes the git diff

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@marionebl marionebl changed the title feat(config-conventional): footer/body-max-line WIP: feat(config-conventional): footer/body-max-line Oct 5, 2018
@marionebl
Copy link
Member

marionebl commented Feb 6, 2020

@Fetz Are still interested in contributing this change?

@marionebl marionebl self-requested a review Feb 6, 2020
@marionebl marionebl self-assigned this Feb 6, 2020
@Fetz
Copy link
Contributor Author

Fetz commented Feb 7, 2020

Hey @marionebl this was blocked since I was getting this error: #437 and you recommended that the fix should be done on conventional-commits-parser.

So I created a ticket and I proposed some solutions and I asked for feedback on what solution should be approached and I even suggested to fix it, but I didn't get any feedback from any of the members...

PS: now that you are a member of conventional-changelog maybe you can see if any of my suggestions are good to fix the issue on ticket so I can unblock this PR.

@marionebl
Copy link
Member

marionebl commented Feb 7, 2020

PS: now that you are a member of conventional-changelog maybe you can see if any of my suggestions are good to fix the issue on ticket so I can unblock this PR.

Ok, thanks for reestablishing the context for me. Let's take the discussion to the conventional-changelog issue then

@marionebl
Copy link
Member

marionebl commented Feb 7, 2020

Thanks for your patience, btw 🙃. I investigated the parser issue a bit.

The scope of it is limited to commits read from COMMIT_EDIT_MSG created via --verbose, right?

Assuming this is true let's move forward with this feature and resolve the --verbose issue independently.

@Fetz
Copy link
Contributor Author

Fetz commented Feb 10, 2020

The scope of it is limited to commits read from COMMIT_EDIT_MSG created via --verbose, right?

Yes it's true :)

let's move forward with this feature and resolve the --verbose issue independently.

Ok I will resolve this week with the conflicts so this PR is free to be merged

update config-conventional with all the configurations
add footer and body max line length rule with the value 100
add tests to all the rules of config-conventional
@Fetz Fetz force-pushed the Fetz:feat/update-config-conventional branch from 745443e to a3ac52c Feb 16, 2020
@Fetz Fetz changed the title WIP: feat(config-conventional): footer/body-max-line feat(config-conventional): footer/body-max-line Feb 16, 2020
@Fetz
Copy link
Contributor Author

Fetz commented Feb 16, 2020

@marionebl I updated the PR:

  • rebased the PR with the latest changes
  • updated the commit to use jest instead of ava

Note:

  • I saw some inconsistency between max length between the header, body and footer (since is only the header showing the current size of the header, should I update those in this PR or another?
  • I saw the v12 tests failed (but it was with the format) maybe is a flaky test?
@marionebl
Copy link
Member

marionebl commented Feb 18, 2020

I saw the v12 tests failed (but it was with the format) maybe is a flaky test?

That was prettier failing due to formatting inconsistencies, pushed the fix

I saw some inconsistency between max length between the header, body and footer (since is only the header showing the current size of the header, should I update those in this PR or another?

Separate PR is fine

@marionebl marionebl merged commit 8b394c9 into conventional-changelog:master Feb 18, 2020
4 checks passed
4 checks passed
ci/circleci: v10 Your tests passed on CircleCI!
Details
ci/circleci: v12 Your tests passed on CircleCI!
Details
ci/circleci: v8 Your tests passed on CircleCI!
Details
ci/circleci: windows-v12 Your tests passed on CircleCI!
Details
@Fetz Fetz deleted the Fetz:feat/update-config-conventional branch Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.