-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
fix: do not generate non-existing paths #459
Conversation
@@ -156,7 +156,7 @@ describe('valid-example', () => { | |||
expect.objectContaining({ | |||
code: 'valid-example', | |||
message: '"self" property type should be array', | |||
path: ['definitions', 'halRoot', '_links', 'self'], | |||
path: ['definitions', 'halRoot', 'example', '_links', 'self'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite tricky. I still need to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of a different issue, fixed in 0e3844f
yet I believe the new error message we generate is actually worse than the former one, or am I wrong?
0e3844f
to
20acd8e
Compare
@@ -169,7 +169,7 @@ describe('valid-example', () => { | |||
expect(results).toEqual([ | |||
expect.objectContaining({ | |||
code: 'valid-example', | |||
message: `"ip_address" property format should match format "${format}"`, | |||
message: `"example" property format should match format "${format}"`, // hm, ip_address is likely to be more meaningful no? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philsturgeon thoughts?
Once we sort this one out, it will be ready to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends? This feels like a limitation of trying to shove everything into a simple string message. We are talking about the example, but it sure could be useful to let them know its the ip_address example that is wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, exactly.
I'd expect to receive something like "ip_address" example property format should match format "${format}"
. Will do it this way.
* fix: do not generate non-existing paths * fix: include example path in valid-example * feat: nicer messages for valid-example rule * test: borrow mocking from feat/custom-functions branch * build: add build to karma
* fix: do not generate non-existing paths * fix: include example path in valid-example * feat: nicer messages for valid-example rule * test: borrow mocking from feat/custom-functions branch * build: add build to karma
Fixes #91
Checklist
Does this PR introduce a breaking change?