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
Extract operation resolvers and execute them after path helpers #152
Conversation
Result of path helper of web framework extension will be resolved by marshmallow extension regardless of the order of extensions.
@yoichi What about this PR is backwards-incompatible? Isn't this just exposing a new hook, or am I missing something? |
I thought extracting a part of functionality of path_helper introduce some incompatibility. But that was my misunderstanding. |
@@ -12,6 +12,7 @@ There are three types of helper functions: | |||
|
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.
Above line will need to be changed to "four".
@yoichi OK, thanks for the clarification. If this doesn't introduce backwards incompatibilities, then I think we should go ahead and merge this. |
docs/writing_plugins.rst
Outdated
|
||
* Definition helpers | ||
* Path helpers | ||
* Operation resolvers |
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.
@yoichi Would it make sense to call these "Operation helpers" for consistency?
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.
Yes. They don't have to resolve something and call them helpers like existing ones seems better. I'll push a change.
Thanks for the suggestion
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.
Just a suggestion for a name change. Other than that, this looks good to merge.
Looks good to me. Thanks! |
Solve #136
NOTE: This fix introduce incompatible change to extension mechanism.
Thanks,