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

feat: Add top-level-interop rule #69

Merged
merged 4 commits into from
Dec 4, 2024

Conversation

hildjj
Copy link
Contributor

@hildjj hildjj commented Dec 3, 2024

A new rule that checks that the top-level item in a JSON file is either an array or an object, as suggested by RFC8259. Not turned on in the recommended config because the software that had this issue is almost entirely obsolete.

Refs #68.

Prerequisites checklist

What is the purpose of this pull request?

As discussed in #68, add a new rule to enforce the top-level item in a JSON doc be an array or object.

What changes did you make? (Give an overview)

Added src/rules/top-level-interop.js and tests/rules/top-level-interop.test.js

Related Issues

Refs #68

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

Should this be in the recommended set? I could be persuaded either way.

A new rule that checks that the top-level item in
a JSON file is either an array or an object, as
suggested by RFC8259.  Not turned on in the recommended
config because the software that had this issue
is almost entirely obsolete.

Refs eslint#68.
Copy link

linux-foundation-easycla bot commented Dec 3, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@hildjj
Copy link
Contributor Author

hildjj commented Dec 3, 2024

I clicked through the CLA.

Copy link
Member

@nzakas nzakas 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 putting this together. Overall, it looks good. Just some areas to clean up before merging.

src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/rules/top-level-interop.js Outdated Show resolved Hide resolved
src/rules/top-level-interop.js Outdated Show resolved Hide resolved
@hildjj
Copy link
Contributor Author

hildjj commented Dec 3, 2024

Made requested changes.

nzakas
nzakas previously approved these changes Dec 3, 2024
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Waiting for a second review before merging.

src/index.js Outdated Show resolved Hide resolved
Co-authored-by: Milos Djermanovic <[email protected]>
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Waiting for @mdjermanovic to re-review.

@mdjermanovic mdjermanovic changed the title feat: Add top-level-interop rule. feat: Add top-level-interop rule Dec 4, 2024
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit af56d6c into eslint:main Dec 4, 2024
16 checks passed
@github-actions github-actions bot mentioned this pull request Dec 4, 2024
@hildjj hildjj deleted the top-level-interop branch December 4, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

3 participants