Add min-body-length option

[soulseekah]

When set an error will be emitted if the body does not contain enough characters. This is useful to require commit bodies with more information, like impact, motivation, approach, testing results, new behavior, etc.

Default: no minimum. Suggested: at least 150.

Fixes #136.

[mristin]

Thanks a lot, @soulseekah ! I won't make it today to review -- hopefully tomorrow.

In the meantime, could you please have a look at the CI -- I think you need to fix the commit message to comply to 72 characters width?

@Fryuni would you mind reviewing as well, so that at least two pairs of eyes looked at the code?

[soulseekah]

Thank you @mristin. The commit validation issue is a false positive. My longest line is 69 characters :)

This is probably related to #111

[mristin]

Thank you @mristin. The commit validation issue is a false positive. My longest line is 69 characters :)

This is probably related to #111

I fixed #111 -- would you mind rebasing your PR so that the tests can run again?

[Fryuni]

@Fryuni would you mind reviewing as well, so that at least two pairs of eyes looked at the code?

Sure, I'll take a look today.

[Fryuni]

I've got one point of preference in the implementation. Besides that, there are no tests for this.

[Fryuni]

Should it could newlines as characters?

The lines are split by \n, so they are not included in bodyLines. But calling join() without any arguments adds a , between each line, effectively counting what was originally the line breaks.

I'd prefer join('') to not count those, but it's definitely not blocking

[soulseekah]

Added the '', good point.

[soulseekah]

While writing tests the '' seems counterintuitive. If the message is laid out into one long string shouldn't there be a space when joining?

  const message =
    'Change SomeClass to OtherClass\n' +
    '\n' +
    'This replaces the SomeClass with OtherClass in all of the module\n' +
    'since Some class was deprecated.';

If we do '' we get 96 characters but the string would become "This replaces the SomeClass with OtherClass in all of the modulesince Some class was deprecated."

Instead of "module since". What would be more intuitive then?

[Fryuni]

I think both counting and not counting the newlines are equally fine as those are mostly corner cases.
But given how unintuitive it might be then counting would be better. So count newlines the same as counting whitespaces

[soulseekah]

Thank you @mristin. The commit validation issue is a false positive. My longest line is 69 characters :)
This is probably related to #111

I fixed #111 -- would you mind rebasing your PR so that the tests can run again?

Rebased.

[soulseekah]

I've added some tests assuming that we join with a space. Happy to agree on whatever you think is better for glueing lines together.

Quick question: should I be running npm run pack for PRs here or should I leave the dist/ directory alone and commit only the src/?

[mristin]

@soulseekah

Quick question: should I be running npm run pack for PRs here or should I leave the dist/ directory alone and commit only the src/?

I think you need to include dist/ for the changes to take place.

[soulseekah]

Added another test. So three tests in total:

• fail on minimum body length
• pass on exact minimum body length (and obviously over the minimum)
• pass on default body length (no minimum)

These assume that the join is with spaces.

[mristin]

Thanks again a lot, @soulseekah for the contribution and @Fryuni for the review!