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

schema: Require that a single note isn’t inside an array #3090

Merged
merged 4 commits into from
Jan 9, 2019

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Nov 14, 2018

This makes the notes field behave the same way as the browser support statement array.

review?(@Elchi3)

@Elchi3 Elchi3 added the schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project. label Nov 16, 2018
@Elchi3
Copy link
Member

Elchi3 commented Nov 26, 2018

Do you have thoughts on #195 ?

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Nov 26, 2018

That allowing single‑element arrays in one context and disallowing them in another is rather inconsistent and we should either allow or forbid them in both contexts.

@Elchi3
Copy link
Member

Elchi3 commented Jan 8, 2019

I think we've more or less settled on forbidding single-element arrays, so I think we should accept this PR. It needs a rebase now, though.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Jan 8, 2019

review?(@Elchi3): I have now resolved merge conflicts and fixed the build errors (please merge this before more issues arise).

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Almost good to go.

"notes": [
"A single note in an array"
]
"notes": "A single note in an array"
Copy link
Member

Choose a reason for hiding this comment

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

This makes no sense now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s because I did a regex replace on the whole repository, and forgot to check for this.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@Elchi3 Elchi3 merged commit 8575f4d into mdn:master Jan 9, 2019
@ExE-Boss ExE-Boss deleted the schema/compat-notes branch January 9, 2019 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants