-
-
Notifications
You must be signed in to change notification settings - Fork 65
Add reporter option
#55
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
Conversation
|
@sindresorhus @transitive-bullshit could you take a look at this? |
index.js
Outdated
|
|
||
| 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
| { | ||
| "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/awesomeusing 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