-
-
Notifications
You must be signed in to change notification settings - Fork 266
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
refactor routes file #575
refactor routes file #575
Conversation
8ddf4a4
to
4a9aa13
Compare
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.
No way for me to check the refactoring, I will accept it in good faith 😄
However: I'm not super happy using the Collection
. I prefer the plain PHP counter parts, because I fear the (unnecessary) overhead in a performance critical part (route file => always loaded on each request, no matter what). Would you mind using array_map
and array_filter
instead?
FTR, I don't mind the Collection but rather in code parts where I absolutely know that performance doesn't matter and I acknowledge the increased readability.
Thanks!
I tried once more to understand the changes of the refactoring but had to give up, no idea. Besides the tests in here I verified it doesn't break some applications I use the library in 👍 Would you mean adding some more explanation in the commit message what / why the refactoring? thanks! |
updated |
super cool. can you squash all commits into and include that actual information in the commit message itself, please? I'll merge it then, thanks! |
The logic for specific controller config for mutation and query was 95% a like. This will also make it easier to add subscription specific routing in the future
dc90e90
to
7e981d2
Compare
ready, if you didn't notice:) |
Didn't 🤷♀️ :-) Thanks! |
https://github.com/rebing/graphql-laravel/releases/tag/5.0.0 has been released which includes this! |
Summary
Refactor of routes file
The logic for specific controller config for mutation and query was 95% a like. So therefore the refactor. This will also make it easier to add subscription specific routing in the future
Type of change