Commitlint: Integrating custom reporters/formatters

[byCedric]

Expected Behavior

At Peakfijn we are using Commitlint for all of our new projects, and we are really happy with it! Right now we are in the process of evaluating and checking on points where it could be improved. One of these points was the ability to report the outcome the CLI command.

Current Behavior

We are using Bitbucket Pipelines are we are testing all commits since start or since when we switched to our new standard. When someone made an error in the commits, we have to scroll and search for the conflicting rule. There is no way in getting a commit range from Pipelines, but that's another discussion.

It would be really really great for us if we can "report" or output the CLI command's result in a specific format. I've noticed that Circle CI, Bitbucket Pipelines and probably others have support for junit reports. With this a nice UI is rendered with all failing tests/assertions.

Affected packages

• [X] cli
• [ ] core
• [ ] prompt
• [ ] config-angular

Possible Solution

I noticed that both ESLint and Stylelint, other linters we use, have support for this. So for a possible solution I would recommend taking a look at both variations. I'm also happy contribute to this feature too, if it's something you and/or others are interested in.

Steps to Reproduce (for bugs)

Context

Your Environment

[marionebl]

Thanks for the nice suggestion @byCedric, I really appreciate it.

I'd like to see support for custom formatters in commitlint. This would involve a number of things:

• Introduce a new cli flag: --format=<formatter-id> | -o=<formatter-id>
• Introduce a new config key for commitlint.config.js: formatter
• formatter-id is a npm module id resolved from the current cwd (or --cwd). It defaults to @commitlint/format
• Resolve the formatter early on during cli runs, fail loudly if not available

Want to lend a hand @byCedric?

[byCedric]

You are welcome @marionebl! As said, we are using it at Peakfijn now for all our projects. It helps us making sure everyone follows our flavor of Angular Conventional Commits Conventions. And it's doing pretty well on Bitbucket (Pipelines) too. Also, I agree with you on all 4 points. It should be configurable through the general (possibly shared) config. But also allow overwriting through CLI.

You make it sound relatively easy, I'm sure I can create a PR for this. And I'm happy to help of course! Basically, I would break it down like this.

1. Expand both the config and CLI with an option format.
2. Resolve the formatter in @commitlint/cli, fail if something is wrong.
3. Use this formatter over here.

I'm still a bit concerned about every message being independently formatted in the CLI. The way I see it this should be sent to the formatter at once, in a list. This allows the formatter to create a "summary", just like eslint's stylish formatter. You could fix this by resolving all messages, in parallel, with lint() before calling the formatter.

I also think the (extra) logging, in the main method in CLI, would potentially "screw up" strict output formats (like JUnit). It might be wise to move this part to the default formatter so you can customize it, per formatter.

I know how I would fix this, but it's always better to discuss things like this 😄 So, before I get dirty with this, what is your take on these two (potential) issues?

[marionebl]

Cool, this gonna be good :)

• formatting 1 or n messages: Let's change to n as suggested and make @commitlint/format accept an array of messages
• Let's get rid of all extra console.log statements. If we want to produce debugging output in the future it should got to stderr

[byCedric]

Sounds like a plan! I'll get started on this then. Based on my schedule I would say I probably have something around Sunday (2018/09/09 UTC+02:00).

[byCedric]

Ok, so I had some time to work on this. It's going pretty well so far, but I'm not there yet! So, here is a quick update. Things I did:

1. Add formatter to @commitlint/load with a default value of @commitlint/format.
2. Add --format|-o to @commitlint/cli with null as default (to use the config formatter).
3. Fail @commitlint/cli if the formatter couldn't be found or loaded.

"Challenges" I found

_1. I'm not sure how the exit code should be generated_
I was a little confused about the current state how the status code is being handled. It looks like @commitlint/cli throws exceptions, per commit message, and adds an error.type containing @commitlint/cli. Finally, this is used to determine the status code 1 (error) or something else.

Should this now be calculated with the resolved reports array instead of these errors? With this change, you can still output "summaries" without erroring out. ESLint also has the approach of checking if there are issues in the results. And XO is doing it too.

Assumptions I made (not all are in the PR right now)

_1. Set default formatter in @commitlint/load instead of @commitlint/cli._
I did this so that whenever --format|-o is provided, it should be used (no matter what). If we put the default value in @commitlint/cli, we would have to check if it's the default. Else it would always override the @commitlint/load formatter.

_2. Pass both the reports and the flags object_
Currently, it uses some options from the flags in the "old" formatter (hardcoded into @commitlint/cli). To allow these kinds of options to be used by formatters, we need to pass them through. Or at least, that's what I think.

_3. Create a "report" object, instead of a simple list, for the formatters_
To fix the challenge with determining the proper exit code, we need to iterate the result items. It might be useful for different formatters to share this data. So, instead of simply sending a "list" of resolved messaged I propose to send an object like this:

{
    /** I noticed `result.valid` is available, so this will be true if _all_ results are valid. */
    valid: true,
    /** The total sum of all errors encountered in the issues, so basically how many rules are violated. */
    errorCount: 9,
    /** Same as error count, but for warnings. */
    warningCount: 23,
    /** A list of all commit messages, resolved with `lint()`. */
    results: [{...}, {...}],
}

---

I'm going to try to fix the challenge today. Feel free to stop me anytime if you think the challenge solution or assumptions shouldn't be done this way. @marionebl 😄

[byCedric]

Again, quick update, I'm done with the issues I listed above. Again, I really made some assumptions so I separated it in a different branch. You can see these changes over here. If you don't have any remarks I can simply merge it into the existing PR.

Hope you like it @marionebl. 😄

Oh, almost forgot, some of the @commitlint/cli tests are actually testing output created by the formatter. I only edited the things that were absolutely necessary, so I kept them there. Maybe that should be cleaned to.

[marionebl]

Thank you so much for this PR. I'll have a look at your changes right away.

Exit codes

Should this now be calculated with the resolved reports array instead of these errors? With this change, you can still output "summaries" without erroring out. ESLint also has the approach of checking if there are issues in the results. And XO is doing it too.

Yes, let's go with the approach already established by ESLint. 👍
I'd like to keep the current error-aggregation, though - so the new order of things would be:

1. Create the report project as described
2. Let the formatter do its thing
3. Decide based on report.valid if a "managed" error (with .type) should be thrown

Default formatter in load

Ok, let's do it this way

Pass both the reports and the flags object

I'd prefer to pass only what we anticipate is needed in the formatters and hide the fact there is concept of cli flags from the formatters. What cli flags do you think might be relevant to formatter implementations?

Report object

Yes let's do this.

Testing output

Yes, some of the CLI tests might become obsolete - I'd take care of this after landing the feature.

[marionebl]

The changes on the "assumptions" branch look fine to me 👍

[byCedric]

Thanks for the checkup! I'll update the exit code mechanism use the managed errors.

So far, the current formatter only requires the flag color to determine if it can use chalk or not. I don't think there will be much more to configure besides this flag. I guess we can simply call it like this, right? @marionebl

const output = format(report, {color:flags.color});

[byCedric]

Ok, i've updated the PR with the following changes:

1. Revert formatter parameters to only report and options. _Only the color flag is being passed as option for now._ #ef76b84 and #32e8e72
2. Throw a managed error (with .type) based on the .valid property of the report. #f2d78c9
3. Update configuration and cli documentation. #565858c and #9fa75f2

I can write something about custom formatters in the documentation, if you want. Recently wrote a CI guide for Expo, if you want something similar for this feature, let me know! 😄

Again, thanks for being awesome!

[byCedric]

Is there anything I can still do for this feature? @marionebl