-
Notifications
You must be signed in to change notification settings - Fork 0
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
Detect and collect deprecated fields in the check #24
Conversation
This pulled in a bunch of crates we do not need. A few of these are crates for networking and async
This comment provides a function to collect deprecated fields from the given check, so they can be logged as an error. Due do the limitations of jsonschema-rs, it is not possible to log deprecations if the given schema is not valid otherwise. I wish to address this via using <https://github.com/santhosh-tekuri/boon> instead of committing this to the upstream project, since the API of boon is way better. Furthermore, it is not possible to detect deprecations in linked types ($ref), due to shortcomings in the used crate. Currently this functionality is not needed and might be addressed via moving to boon
It is not necessary to utilise the Result type here and complicates the code. Therefore, I removed it while implementing the collection of deprecations
There was some unformatted code in the upstream branch. Maybe having a check for this could come in handy
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.
I like the approach catching every deprecated field, I would have aimed for something way simpler. Also, I would prefer the deprecation thing to be another validator, not modifying too much what we already had in place.
Can you fix the tests?
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.
- Have you considered locking down on a more recent jsonschema meta-schema version ? I understand more recent versions have a richer validation/schema-evolution capabilities.
- I wonder if it's interesting/feasible to express deprecation of fields using the schema-evolution capabilities of more recent versions. 🤔
Yes I did, but updating the crate is not required, since it does not implement handling deprecations in future releases. As stated in the commit messages, we're going to replace this crate in future releases with boon, for it actually covers modern specs and bumping the crate to 0.20 requires updating the code either way. |
I was not suggesting to update the crate, necessarily. However, I am/was under the impression that more recent versions of the json schema spec (not the crate, but the "dialect" that a specific schema instantiation adheres to) support field-level deprecation (according to the link I shared previously). I also understand that newer versions of the jsonschema crate support newer dialects (like draft2020, draft2019, etc.) but I am not fully aware of specific dialect support/conformance limitations of the current jsonschema crate. Concretely, I was wondering :
|
Yeah they do and that is what is leveraged here
TLDR: they don't support this
Sure, I can replace the link, but it won't change the fact that this implementation would still be required
I don't think it is wise to have multiple sources of truth As reference af224c0 |
Indeed it is not, I agree. But how is marking specific fields in the DSL spec as deprecated an instance of multiple sources of truth? Isn't the spec supposed to be the source of truth and tlint the "verifier"/checker of an instantiation of a check against the spec? |
This PR allows to collect and show deprecations in the checks. For more information and reasoning behind changes, have a look at the commit message. (I wish I could just tell GitHub to throw these in here)