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

Update: pass rule meta to formatters RFC 10 #11551

Merged
merged 6 commits into from Mar 30, 2019

Conversation

EasyRhinoMSFT
Copy link
Contributor

@EasyRhinoMSFT EasyRhinoMSFT commented Mar 25, 2019

What is the purpose of this pull request? (put an "X" next to item)

[X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[X] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[X] Add something to the core
[ ] Other, please explain:

https://github.com/EasyRhinoMSFT/rfcs/blob/master/designs/2019-expose-rules-to-formatters/readme.MD

What changes did you make? (Give an overview)

This change adds a new optional argument to the interface function exported by formatters. The argument is a dictionary that is keyed with the ruleIds of each rule run during the analysis. The entry values are the corresponding rule's meta property.
This change also adds a new JSON formatter that serializes both the results array and the new data object argument.
Other changes include unit test coverage for the new JSON formatter as well as updates to the CLI unit tests.
Finally, this updates the HTML formatter + tests, and fixes a bug that caused invalid URLs to be written out.

Is there anything you'd like reviewers to focus on?

Please review the changes to the working-with-custom-formatters document. I want to make sure they correctly follow the existing style, structure, and voice.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Mar 25, 2019
@EasyRhinoMSFT EasyRhinoMSFT changed the title Update: pass rule meta to formatters (RFC #10) Update: pass rule meta to formatters RFC #10 Mar 25, 2019
@EasyRhinoMSFT EasyRhinoMSFT changed the title Update: pass rule meta to formatters RFC #10 Update: pass rule meta to formatters RFC 10 Mar 25, 2019
Copy link
Member

@not-an-aardvark not-an-aardvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! This looks good overall -- I have a few minor suggestions.

lib/formatters/html.js Outdated Show resolved Hide resolved
docs/developer-guide/working-with-custom-formatters.md Outdated Show resolved Hide resolved
docs/developer-guide/working-with-custom-formatters.md Outdated Show resolved Hide resolved
docs/developer-guide/working-with-custom-formatters.md Outdated Show resolved Hide resolved
lib/formatters/html.js Show resolved Hide resolved
lib/formatters/json-with-metadata.js Outdated Show resolved Hide resolved
lib/formatters/json-with-metadata.js Outdated Show resolved Hide resolved
@not-an-aardvark not-an-aardvark added accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint and removed triage An ESLint team member will look at this issue soon labels Mar 27, 2019
@ilyavolodin ilyavolodin merged commit bc3e427 into eslint:master Mar 30, 2019
@EasyRhinoMSFT EasyRhinoMSFT deleted the rfc-10 branch March 30, 2019 02:10
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 27, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants