Skip to content

move extractors and generic schema utilities out of handler.rs#554

Merged
davepacheco merged 7 commits into
mainfrom
extractor-move
Jan 11, 2023
Merged

move extractors and generic schema utilities out of handler.rs#554
davepacheco merged 7 commits into
mainfrom
extractor-move

Conversation

@davepacheco

@davepacheco davepacheco commented Jan 9, 2023

Copy link
Copy Markdown
Collaborator

I felt like handler.rs was getting a bit big and full of unrelated stuff. Here I'm separating the extractors into a submodule with the eventual intent on putting different ones into different files within extractors. This involved moving some functions used by both handler.rs and the new extractor module into schema_util.rs (analogous to http_util.rs).

I believe nothing has changed in this PR except for moving code from one file to another and adjusting imports / "mod" statements.

This depends on #553 -- will update the base branch once that one lands and I'll keep this in "draft" for the meantime.

@davepacheco davepacheco requested a review from ahl January 9, 2023 21:59
Base automatically changed from style-update to main January 9, 2023 22:33
@davepacheco

Copy link
Copy Markdown
Collaborator Author

Sorry for the force push and general noise.

@davepacheco davepacheco marked this pull request as ready for review January 10, 2023 00:07

@ahl ahl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I really like this reorganization. I said "oooh" aloud when I saw that extractor was its own mod--like it a lot.

api_description.rs has a bunch of j2oas_ prefixed functions; would those make sense to move into schema_util.rs? No or Not now are fine answers.

@davepacheco

Copy link
Copy Markdown
Collaborator Author

Thanks for the review.

api_description.rs has a bunch of j2oas_ prefixed functions; would those make sense to move into schema_util.rs? No or Not now are fine answers.

That looks like a good idea. I will do it in a follow-up PR.

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