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 multipart request for play #500

Merged
merged 20 commits into from
Dec 4, 2020
Merged

add multipart request for play #500

merged 20 commits into from
Dec 4, 2020

Conversation

igorfialko
Copy link
Contributor

…hostdogpr-master

# Conflicts:
#	adapters/play/src/main/scala/caliban/PlayAdapter.scala
#	adapters/play/src/main/scala/caliban/PlayRouter.scala
build.sbt Outdated Show resolved Hide resolved
@paulpdaniels
Copy link
Collaborator

@igorfialko are you planning to continue work on this?

@igorfialko
Copy link
Contributor Author

@paulpdaniels Looks like the only thing left for play is the usage of other testing library. Is that correct?

@paulpdaniels
Copy link
Collaborator

That was all I had yes @igorfialko

@igorfialko
Copy link
Contributor Author

@paulpdaniels Ok, I'll try to squeeze it this week. Maybe further splitting #451 by framework would make sense?

@paulpdaniels paulpdaniels mentioned this pull request Oct 13, 2020
5 tasks
adapters/play/src/main/scala/caliban/Uploads.scala Outdated Show resolved Hide resolved
adapters/play/src/main/scala/caliban/Uploads.scala Outdated Show resolved Hide resolved
adapters/play/src/main/scala/caliban/Uploads.scala Outdated Show resolved Hide resolved
adapters/play/src/test/scala/caliban/AdapterSpec.scala Outdated Show resolved Hide resolved
adapters/play/src/test/scala/caliban/AdapterSpec.scala Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
Copy link
Collaborator

@paulpdaniels paulpdaniels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small nits from me otherwise LGTM

@igorfialko igorfialko closed this Nov 8, 2020
@igorfialko igorfialko reopened this Nov 8, 2020
Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a tiny remark on dependencies that can be simplified. Also I wonder if there's an easy way to make the dependency to Blocking and Random optional without duplicating code? If not this is fine.

build.sbt Outdated Show resolved Hide resolved
@paulpdaniels
Copy link
Collaborator

I do recall there is a +++ operator that lets you select between a provided left or right environment, but I don't know if that would apply here because we actually require the Runtime upfront so we wouldn't be able to switch it out.

Copy link
Owner

@ghostdogpr ghostdogpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this!

@ghostdogpr ghostdogpr merged commit fb7b2c8 into ghostdogpr:master Dec 4, 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

3 participants