-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add reporter
option
#55
Conversation
@sindresorhus @transitive-bullshit could you take a look at this? |
index.js
Outdated
@@ -69,7 +73,11 @@ lint.report = async options => { | |||
process.exitCode = 1; | |||
|
|||
file.path = path.basename(file.path); | |||
console.log(vfileReporterPretty([file])); | |||
if (options.customReporter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The option name can simply be named reporter
.
index.js
Outdated
console.log(options.customReporter([file])); | ||
} else { | ||
console.log(vfileReporterPretty([file])); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be simplified to:
const reporter = options.reporter || vfileReporterPretty;
console.log(reporter([file]));
package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "awesome-lint", | |||
"version": "0.8.0", | |||
"version": "0.9.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't bump version in a pull request. It's up to the maintainers when, how, and what version to bump :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my apologies, that makes sense.
Yeah, makes sense to have this. The option must be documented in the readme and needs a test in https://github.com/sindresorhus/awesome-lint/blob/master/test/cli.js |
reporter
option
It should be exposed as a CLI flag too. |
@sindresorhus I went ahead and responded to all feedback, let me know if you think there's any additional work to do before merging. |
Looks good :) |
I currently am trying to add CI to
sindresorhus/awesome
using Azure Pipelines, and I would like to use a custom reporter so that I can display test results in the Pipelines test UI. This PR enables that by adding support for custom reporters.PR here: sindresorhus/awesome#1505