-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Support of type hinted scope parameters #415
Support of type hinted scope parameters #415
Conversation
@50bhan Are you going to follow up on this? Else I'd like to offer to help. |
@michapietsch Thanks for the offer. Actually I think this feature is done and ready to merge. But doing what is up to @freekmurze and @AlexVanderbist. Maybe there is a policy for adding new features! I would appreciate if they enlighten us. |
Hey, my bad fir not following this up sooner. I'll look into this PR when im at the office later today. Thanks! |
@50bhan Thanks for your work and sorry for rushing forward |
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.
Great start! Couple of thoughts and nitpicks I've added in the comments. Also, can we add another test that has a scope with both regular parameters and scoped parameters?
Thanks for the PR! 👍
…ravel-query-builder into feat/type-hinted-scope-support
@AlexVanderbist All changes have been done. Please check it out and let me know if something else is needed. |
@50bhan thanks for the changes, everything looks great. One final thought regarding the (non-)numeric keys: would it make more sense to use |
@AlexVanderbist Thanks. It's way better now. |
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.
Awesome, thanks! Will merge this in a couple of minutes 💯
Add ability of resolving type hinted scope parameters (issue #146)