-
Notifications
You must be signed in to change notification settings - Fork 121
feat: handle deprecated #79
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Carlos Alexandro Becker <[email protected]>
pending invopop/jsonschema#79 the jsonschema should also have the correct deprecated markings Signed-off-by: Carlos Alexandro Becker <[email protected]>
|
Thanks! Tests required. This probably wants to be supported in the array items also. |
|
@samlown thanks, done! |
|
@samlown merged, fixed conflicts |
|
gently ping @samlown |
stefanhaller
left a comment
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.
Awesome feature, let me express my interest in having this.
Just one thought about the tag format below.
|
|
||
| // Tests for deprecated | ||
| DeprecatedField string `json:"deprecated_field" jsonschema:"deprecated=true"` | ||
| DeprecatedSlice []string `json:"deprecated_slice" jsonschema:"deprecated=true"` |
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 find it surprising that you have to add =true here. When would anybody ever set this to false? I would have expected that you just say jsonschema:"deprecated", like you do e.g. for json:"omitempty".
) - **PR Description** This removes generated Config.md entries that are contain the word: `deprecated` (case insensitive) anywhere in their header comment. I think adding in deprecated configs into the reference config clutters it at best, and encourages users to use those configs at worst. They are still going to be validated by the schema, but this PR removes them from the generated config. I also added in the missing `deprecated` yaml tags in our user config in anticipation of invopop/jsonschema#79 getting merged.
this allows to set the
jsonschema:"deprecated=true"tag