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

Sort object types before writing markdown file #874

Merged
merged 1 commit into from
Jun 8, 2022
Merged

Conversation

jeskew
Copy link
Contributor

@jeskew jeskew commented Jun 8, 2022

This PR sorts the object types written to generated markdown files alphabetically by name. It's an attempt to make PRs that alter what qualifies as a resource (e.g., #805) easier to review.

Currently, the order in which types appear in generated markdown files is determined by when they are used in a provider's swagger model. As a consequence, when a PR expands the definition of what qualifies as a resource, that can cause shapes to change position in the .md file, leading to a bunch of noise in the resulting diff, e.g.:

image

With a stable sort, the diff for .md files would just be insertions.

@jeskew jeskew requested a review from anthony-c-martin June 8, 2022 14:03
@jeskew
Copy link
Contributor Author

jeskew commented Jun 8, 2022

I'm taking a look over the generated types from this (in #871) to make sure nothing unexpected is happening. If there's nothing surprising there, will merge and then run the type update workflow on main.

@jeskew
Copy link
Contributor Author

jeskew commented Jun 8, 2022

All the log.out files in #871 had zero surprises, and I spot checked several markdown files to make sure objects were just being moved around. Everything looks hunky dory.

@jeskew jeskew merged commit 0b5a875 into main Jun 8, 2022
@jeskew jeskew deleted the jeskew/md-stable-sort branch June 8, 2022 17:54
@majastrz
Copy link
Member

majastrz commented Jun 9, 2022

This is a great change btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants