Skip to content
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

Add test trait \Nuwave\Lighthouse\Testing\MakesGraphQLRequestsLumen for usage with Lumen #1100

Merged
merged 9 commits into from Jan 23, 2020

Conversation

rafaelmarcelino19
Copy link
Contributor

#1094
TestResponse is not available in lumen I added this class with the same methods of MakesGraphQLRequests to work in Lumen

@spawnia
Copy link
Collaborator

spawnia commented Jan 10, 2020

Thanks for the PR, i really appreciate it.

What's preventing us from using a single class for both?

@spawnia
Copy link
Collaborator

spawnia commented Jan 22, 2020

This PR seems to have no changes. Are you planning any?

@rafaelmarcelino19
Copy link
Contributor Author

sorry, i didn't add the changes

@spawnia
Copy link
Collaborator

spawnia commented Jan 22, 2020

What's preventing us from using a single class for both?

I was actually asking a question here, not necessarily suggesting we should split it. Thanks for giving it a shot, though.

Considering the two approaches, i actually like having separate classes better. Otherwise the helper kind of becomes "worst of both worlds", with watered down type hints and little workarounds. I tried abstracting further and basing both on a single base trait and found that this complicates things even further.

So, i finally settled on your original approach and added the separate trait back in. Tweaked it a little and added docs and a changelog entry. Let me know if that works for you.

@rafaelmarcelino19
Copy link
Contributor Author

for me ok, I thought you didn't want to keep two classes separate, but that would really become "the worst of both worlds"

@spawnia spawnia changed the title Add class MakesGraphQLRequestsLumen for test work in Lumen Add test trait \Nuwave\Lighthouse\Testing\MakesGraphQLRequestsLumen for usage with Lumen (#1100) Jan 23, 2020
@spawnia spawnia changed the title Add test trait \Nuwave\Lighthouse\Testing\MakesGraphQLRequestsLumen for usage with Lumen (#1100) Add test trait \Nuwave\Lighthouse\Testing\MakesGraphQLRequestsLumen for usage with Lumen Jan 23, 2020
@spawnia spawnia merged commit d0085aa into nuwave:master Jan 23, 2020
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.

None yet

2 participants