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

fix: Update how schema is ordered #3399

Merged
merged 6 commits into from
Oct 17, 2022
Merged

fix: Update how schema is ordered #3399

merged 6 commits into from
Oct 17, 2022

Conversation

sjrl
Copy link
Contributor

@sjrl sjrl commented Oct 17, 2022

Related Issues

  • Json schema ordering

Proposed Changes:

  • Used the sort_keys=True option in the json.dump function to order the keys
  • Ordered the anyof_list which was causing the order of the "type": "boolean" and "type": "string" to switch depending on which system the schema generation was run

How did you test it?

  • Got it to pass the CI schema validation

Notes for the reviewer

Checklist

@sjrl sjrl marked this pull request as ready for review October 17, 2022 11:39
@sjrl sjrl requested a review from a team as a code owner October 17, 2022 11:39
@sjrl sjrl requested review from julian-risch and removed request for a team October 17, 2022 11:39
@sjrl sjrl added topic:DX Developer Experience topic:pipeline labels Oct 17, 2022
Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍 Using the json.dump(sort_keys=True) is a clean solution. The only thing I am wondering about are the schema changes that drop embed_meta_fields, verify_certs, username. This is not in sync with the current main branch. Please double-check what's the issue with these before merging. I don't understand where these changes come from.

@sjrl sjrl merged commit ca2a1e1 into main Oct 17, 2022
@sjrl sjrl deleted the json-schema-fix branch October 17, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:DX Developer Experience topic:pipeline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants