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

linter error message is confusing #3256

Closed
wbamberg opened this issue Jan 4, 2019 · 3 comments · Fixed by #15791
Closed

linter error message is confusing #3256

wbamberg opened this issue Jan 4, 2019 · 3 comments · Fixed by #15791
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files.

Comments

@wbamberg
Copy link
Collaborator

wbamberg commented Jan 4, 2019

If an identifier contains disallowed characters, then the linter's message is:

   item['path']['to']['item']['parent'] should NOT have additional properties

This is wrong and misleading, and it would be much more helpful to say something like:

   item['path']['to']['item'] key contains disallowed characters

...even better if it could link to docs for allowed characters, but https://github.com/mdn/browser-compat-data/blob/master/schemas/compat-data-schema.md does not seem to document this.

@ddbeck ddbeck added the linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. label Jan 4, 2019
@wbamberg
Copy link
Collaborator Author

wbamberg commented Jan 4, 2019

Actually it's not wrong, it's just unhelpful.

The schema says https://github.com/mdn/browser-compat-data/blob/master/schemas/compat-data.schema.json#L197-L206, i.e. "identifiers can only contain '__compat' or properties matching that regex, no other properties".

ajv then applies that to the data and says "wait, there are other properties", which is correct but usually not a helpful error message.

@Elchi3
Copy link
Member

Elchi3 commented Jan 7, 2019

We might want to look for upstream issues on this or file this issue upstream with ajv.

@Elchi3 Elchi3 added this to Linter improvements in Non-data issue overview Jan 10, 2019
@ddbeck ddbeck added this to Tooling and broad data quality improvements in Prioritization review Sep 4, 2020
@queengooborg
Copy link
Collaborator

queengooborg commented May 11, 2021

It's been a while since we've looked into this one. Looking around, I found that ajv has another package that allows for adding custom error messages on schema failure: ajv-errors. Assuming it plays well with better-ajv-errors, I think this could solve our issue. My pull request for better-ajv-errors was merged to ensure that we can use both of them, so we can now add a custom message!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants