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.

Sign up for GitHub

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

Jump to bottom

Java complete directive(s) should take arguments of type ResponseEntity instead of RequestEntity #641

Closed
hseeberger opened this issue Dec 11, 2016 · 6 comments
Closed

Java complete directive(s) should take arguments of type ResponseEntity instead of RequestEntity #641

hseeberger opened this issue Dec 11, 2016 · 6 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted bug help wanted Identifies issues that the core team will likely not have time to work on
Projects
Bug Hunting
Milestone
10.0.6

Comments

@hseeberger
Copy link
Contributor

This extends to Marshaller[T, RequestEntity].

Notice that a RequestEntity is a ResponseEntity and not the other way round!

With the current implementation, it's not possible to write a marshaller which marshals to CloseDelimited, because that's nole a ResponseEntity and not a RequestEntity.
@jrudolph jrudolph added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted bug help wanted Identifies issues that the core team will likely not have time to work on labels Dec 12, 2016
@jrudolph
Copy link
Member

Looks like an oversight to me, left over from the Java port.

@hseeberger hseeberger mentioned this issue Dec 12, 2016
Open
@jrudolph jrudolph added this to Backlog in Bug Hunting Feb 27, 2017
@jrudolph jrudolph moved this from Backlog to Low-Hanging-Fruit in Bug Hunting Feb 27, 2017
@btomala
Copy link
Contributor

btomala commented Mar 15, 2017

If no one working on it I will take a look on it. Looks pretty simple.

@jlprat
Copy link
Member

jlprat commented Mar 16, 2017

Just wondering, isn't this change a potential breaker for users of those classes? Basically, is it binary and source compatible?

@btomala
Copy link
Contributor

btomala commented Mar 20, 2017

@jlprat It is source compatible but isn't binary. And it is very simple change. Do you want this change even though?

@jlprat
Copy link
Member

jlprat commented Mar 20, 2017

Yes, the ticket is up to be picked. It would be cool if you could provide a PR! 😃

jrudolph added a commit that referenced this issue Apr 26, 2017
@jrudolph
+htp #641 Add complete directive overload that take ResponseEntity …
28fdadd 
…as a parameter (#982)

The new overload allows to call `complete` with `CloseDelimited` entities (which is
a `ResponseEntity` but no `RequestEntity`.
@jrudolph jrudolph mentioned this issue Apr 26, 2017
Merged
@jrudolph jrudolph added this to the 10.0.6 milestone Apr 26, 2017
@jrudolph
Copy link
Member

Fixed in #982.

@jonas jonas moved this from Low-Hanging-Fruit to Done in Bug Hunting May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted bug help wanted Identifies issues that the core team will likely not have time to work on
Projects
No open projects
Bug Hunting
Done
Milestone
10.0.6
Development

No branches or pull requests
4 participants
@jrudolph @hseeberger @jlprat @btomala