Skip to content

New --alphabetize switch: Sorts output file consistently #942

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

Closed

Conversation

MrLeebo
Copy link
Contributor

@MrLeebo MrLeebo commented Sep 23, 2022

Screen Shot 2022-09-24 at 3 47 34 PM

This PR adds a new --alphabetize switch that sorts the entries in the typescript output file alphabetically. This ensures the typescript file appears in a consistent sort order. Sometimes with dynamically generated docs, the order of the operations can change just depending on the timing of when each controller gets parsed. This can create trouble if you're checking your types into git. It can be difficult to review what changes were actually made when the only thing you can see in git diff is all of the operations being shuffled around.

@MrLeebo MrLeebo marked this pull request as ready for review September 24, 2022 20:52
@MrLeebo MrLeebo changed the title Alphabetically sort output from transforms New --alphabetize switch: Sorts output file consistently Sep 24, 2022
Copy link
Contributor

@drwpow drwpow left a comment

Choose a reason for hiding this comment

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

Love to see PRs like this! I’m in favor of this merging, but I’d like to see the tests moved out of snapshots and into unit tests where it makes sense.

@MrLeebo MrLeebo requested a review from drwpow October 9, 2022 08:43
@MrLeebo
Copy link
Contributor Author

MrLeebo commented Oct 27, 2022

@drwpow is there anything else I can do to help move this along?

@drwpow
Copy link
Contributor

drwpow commented Oct 29, 2022

This looks great! Sorry for the delay; will merge this today.

Just a heads up: this will be released in the 6.0 version along with some OpenAPI 3.1 additions and a breaking change.

@drwpow
Copy link
Contributor

drwpow commented Nov 2, 2022

Merged exactly as-is in #966. Just fixed the test error & merge conflict but made no changes.

@drwpow drwpow closed this Nov 2, 2022
@drwpow
Copy link
Contributor

drwpow commented Nov 2, 2022

@all-contributors please add @MrLeebo for code, test

@allcontributors
Copy link
Contributor

@drwpow

I've put up a pull request to add @MrLeebo! 🎉

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.

2 participants