-
-
Notifications
You must be signed in to change notification settings - Fork 35
refactor!: extract framework core to cot-core #444
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: master
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
a799923 to
c1c890a
Compare
|
The nightly error should be fixed by rust-lang/rust#150939 |
0033822 to
ead28ff
Compare
m4tx
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.
Looks nice, and seems like it lays a solid foundation for our eventual extraction of the ORM to a separate crate!
I've tried to generate the docs with just docs and it seems like there's a ton of warnings coming from cot-core (outside of missing examples, which are expected) - please make sure these are fixed before we consider merging this.
cot-core/src/handler.rs
Outdated
| } | ||
|
|
||
| /// Private marker type used to distinguish `FromRequest` parameter position in | ||
| /// trait impls. This is used instead of `()` to avoid coherence conflicts. |
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.
Can you elaborate more about this? What's the exact issue this is trying to solve?
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.
During refactor cargo would complain about double implementations, but I was able to remove it now and it works. I suppose it was because the refactor was not finished. I removed it.
| Self::build_error_data(&mut error_data, error); | ||
| } | ||
| Kind::WithMessage(message) => { | ||
| Kind::WithMessage { 0: message, .. } => { |
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.
Perhaps it makes sense to change this enum variant to be a struct variant with named field rather than enum variant? After all, we might (however unlikely) want to add some more fields to it, at which point we'll probably need to add some naming so it's clear what they do. Think "future-proofing the APIs".
This PR attempts to extract the core functionality of the framework to separate crate to speed up compilation among other reasons.
The modules fully/partially moved:
bodyerrorheadershtmljsonresponserequestmiddlewarehandlerI think moving
routerwould be valid move as well, but in the current format it's too intertwined with OpenAPI functionality (that we don't want to pull into core) that it can be done at a later time.With
cot-corebeing a separate crate, the following items had to become public:cot_core::error::backtracecot_core::error::error_impl::*Error.backtrace()NotFound.router()Backtrace.frames()StackFrame.symbol_name()StackFrame.location()StackFrameBody.axum()Body.wrapper()Body.innerBodyInnerInvalidContentTypeAppNameRouteNameBoxRequestHandlerinto_box_request_handler