Major refactor of plugin and support of rules selection

[AdrieanKhisbe]

Hello @azeemba :)

First thanks of all for this useful plugin!

Here is roughly what PR consist in:

• update the code to es6 standard
• set up eslint and prettier for the plugin itself
• add possibility to select rules to check for. (see ddad466)
  • plugin is disabled by default, rules have to be enable as error or warning
  • a all rule is available (that can be configure to ignore comments)
  • rule name is json/the-error-code-in-kebab-case
• adds integrations tests

Builds of branch can be seen on travis, and library is available for test on npm under eslint-plugin-json-beta as [email protected]

Documentation needs to be updated, which I will do after the first feedbacks :)

[azeemba]

@AdrieanKhisbe thanks for the PR!

Due to the size of the PR and some time constraints, it will take me couple of weeks to go through this PR. Hopefully that's okay for you.

If everything looks good, would you be interested in maintaining (or helping maintain) this project as well?

[AdrieanKhisbe]

Your welcome @azeemba ! :)

No hurry! Take the time needed :)

(And yes, might be interested into giving a hand to main it)

[azeemba]

This is awesome, thanks again for your PR!

I have added a suggestion for the integration test and also a question about how "allowComments" is configured right now.

Do you think you can also update the README.md with the information of the rules/config options that are supported?

Since the changes here are significant, I will make a new major release once we land this changeset.

[azeemba]

Should this file have comments?

Suggested change

	"hello": "world"
	"hello": "world" // this is a comment

[AdrieanKhisbe]

yep, that's the point. (jsonc for json comment, as it's used by some.
I can rename it to make it more clear if you want :)

[AdrieanKhisbe]

indeed a comment was missing 🤦‍♂

[azeemba]

After adding a comment in jsonc.json, I had to make this change as well for comments to be allowed.

Suggested change

	"json/json": ["warn", {"allowComments": true}]
	"json/*": ["warn", {"allowComments": true}]

Is it possible for the allow-comments to be just another rule though? Instead of an option on the base rule?

[AdrieanKhisbe]

It can be specificaly set with json/comment-not-permitted.

By default, json/json (and it's alias json/*), activate this rules (as comments are not part of the json spec). the option allowComments allow user to activate everything but this rule.

But it's strange you had to made the change since json/json and json/* are supposed to be the same
I'm going to try to replicate

[AdrieanKhisbe]

Sure I'll update the README accordingly :)
(And sure, I think also a major is best suited for theses kind of chaneges)

[AdrieanKhisbe]

@azeemba I took care of the first adjustments, I'll update the documentation as soon as possible. :)

[AdrieanKhisbe]

@azeemba I updated the documentation.

Tell me if it suits you :)

Here is some things I'd like to do If you agree,

• set up coverage with codecov. (computation, publish, and badge)
• rename lib to src
• add a LICENSE file to the root (it is even mention by the plugin itself https://github.com/azeemba/eslint-plugin-json/blob/master/lib/index.js#L5).
  MIT maybe, unless you prefer ISC that was probably npm init default.

[azeemba]

All those suggestions sound great!
Regarding codecov, we have access to GitHub Actions so we can switch to using that as well if you are interested. Though probably its best to that on a separate PR.

I think having integration tests like you have provided are super valuable but I think there is some bug in the script at the moment. Currently json-with-comments is passing the integration test but it should be failing since .eslintrc has not been updated with the updated filename.

If its okay with you, I would like to add a commit to switch the integration test script to be in JS so the tests can be more structured.

Stale

[azeemba]

Let me know if you have any concerns with the switch in the integration tests!

[AdrieanKhisbe]

No problem @azeemba ! :)
Good work with a35309b

Will then do the suggestions you accepted,
and possibly split the real integration test you created to have the engine, and just the test in a dedicated file with this

Will have a look of github actions. Sounds promising but haven't try it for now. This could be the opportunity :)

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@e2169d9). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff           @@
##             master    #26   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?      1           
  Lines             ?     50           
  Branches          ?      0           
=======================================
  Hits              ?     50           
  Misses            ?      0           
  Partials          ?      0

Impacted Files	Coverage Δ
src/index.js	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2169d9...65158c9. Read the comment docs.

[AdrieanKhisbe]

Hey @azeemba :)
I just implemented the suggestions I made last time.

We might be good to go :)

[azeemba]

Thanks for all your contributions!

Since you have merge privileges now, I will let you merge the PR. I will push out a new major release after that!

[AdrieanKhisbe]

And here it is then :)