mulesoft-labs / raml-jaxrs-codegen

Tools to enable RAML-first development in JAX-RS projects
Other
40 stars 44 forks source link

When MimeType is 'application/octet-stream' generates InputStream #33

Closed justin-calleja closed 10 years ago

justin-calleja commented 10 years ago

When MimeType is 'application/octet-stream' generates InputStream parameter (which reads bytes[] as opposed to Reader which reads char[]).

Includes MediaTypeGeneratorTestCase which is a copy of GeneratorTestCase but loads core/src/test/resources/org/raml/mediatype/application_octet-stream.yaml instead and checks that the generated interface takes an InputStream param.

ddossot commented 10 years ago

A very important pull request, hopefully someone at MuleSoft will notice it...

petrochenko-pavel-a commented 10 years ago

Integrated into merged repo for two projects

ddossot commented 10 years ago

@petrochenko-pavel-a What are the two projects you're talking about? This PR is not merged into the current repo's master.

ddossot commented 10 years ago

@usarid Can you shed some light on the pull request merging strategy for this project? I see some PR being closed, other being commented as being merged (like this one) but clearly it's not merged into this repo.

So: is there another repo where this project is being maintained?

justin-calleja commented 10 years ago

I'm also confused about this.

ddossot commented 10 years ago

@justin-calleja Not sure what to think about what's going on here: either a branch hasn't been pushed, or there's another repo where this project is maintained. The latter would be bad form re:playing the open source game :crying_cat_face:

usarid commented 10 years ago

Hey guys, trying to sort this out, stay tuned...

On Tue, Sep 9, 2014 at 8:00 AM, David Dossot notifications@github.com wrote:

@justin-calleja https://github.com/justin-calleja Not sure what to think about what's going on here: either a branch hasn't been pushed, or there's another repo where this project is maintained. The latter would be bad form re:playing the open source game [image: :crying_cat_face:]

— Reply to this email directly or view it on GitHub https://github.com/mulesoft/raml-jaxrs-codegen/pull/33#issuecomment-54982110 .

ddossot commented 10 years ago

@usarid Very appreciated, thank you :sweat_smile:

justin-calleja commented 10 years ago

:beer:

petrochenko-pavel-a commented 10 years ago

Thanks for your contribution. We have already incorporated these to the project.

justin-calleja commented 10 years ago

Hi petrochenko-pavel-a.

Thanks for pulling in this change in e28c10b564c5176ad1e63fcc14522fb81d23cbae.

Note that the test: core/src/test/java/org/raml/jaxrs/codegen/core/MediaTypeGeneratorTestCase.java

depends on core/src/test/resources/org/raml/mediatype/application_octet-stream.yaml

which I don't see committed in e28c10b564c5176ad1e63fcc14522fb81d23cbae (or the master branch as it currently is).

I'm mentioning this just in case this was missed as I see the test class was committed but the relevant methods commented out (perhaps you ran - it failed because the YAML file is missing - and you commented out to get it to pass).

Btw, I admit that test seems to be more liability than profit. As in there's a lot of setting up just to test that that an InputStream parameter is generated. So, up to you guys if you want to remove it (but currently it's not doing anything, and if uncommented, MediaTypeGeneratorTestCase will fail).

Regards, Justin

petrochenko-pavel-a commented 10 years ago

perhaps you ran - it failed because the YAML file is missing - and you commented out to get it to pass). No there is another misconfiguration which makes tests failing currently. Working on it.

petrochenko-pavel-a commented 10 years ago

And yes I forgot to add file, when replicated it in this repo.

Thanks a lot for noticing.

ddossot commented 10 years ago

What do you mean by "replicated in this repo"? You're not using git merging?

justin-calleja commented 10 years ago

@petrochenko-pavel-a, you're welcome :smile:

@ddossot - agreed, merging would avoid such problems

usarid commented 10 years ago

Hey guys, the picture is just this: after talking to a bunch of people, we feel it'll give a better user experience, and will be clearer, if we combine RAML --> JAX-RS and JAX-RS --> RAML in a single repo that gives you functionality in both directions and in consistent ways. So we've been working on such a repo separately, so as not to confuse people on this repo. We're trying to bring it back together now, and ran into a few hiccups, including what seems to be a bit of a gnarly merge. While we're doing that, we've gotten your extremely valuable contributions, as well as others. To accept a PR and just merge it in we need the Contributor's License Agreement accepted by the author of the PR, so we can keep the licensing clean, as it says in the readme. I believe that didn't happen, and instead a similar or even identical change was just recreated in the project, but as I try to understand what's going on (travel schedules are interfering a bit). We'll sort this out by Monday of not sooner. Thanks for your patience.

justin-calleja commented 10 years ago

Hi usarid,

Thanks for clearing things up. Cool, there's a JAX-RS -> RAML converter! Awesome, thanks for all the work you and everyone else put into these tools :smile: - really handy stuff!

Btw, regarding the license agreement, I had agreed to that: https://gist.github.com/justin-calleja/398d9c9c512a7703b786

Thanks again!

Regards, Justin

ddossot commented 10 years ago

Thanks for chiming in URI. I understand the "merged to two repos" thing now :smile:

What would be super duper great would be to have a CI build for this new consolidated project, with artifacts pushed to MuleSoft's Nexus. That way, using the official version of these plugins would be a no-brainer.

Thanks again. David

usarid commented 10 years ago

Thanks a lot Justin, that actually really helps,

Uri On Sep 13, 2014 2:02 AM, "justin-calleja" notifications@github.com wrote:

Hi usarid,

Thanks for clearing things up. Cool, there's a JAX-RS -> RAML converter! Awesome, thanks for all the work you and everyone else put into these tools [image: :smile:] - really handy stuff!

Btw, regarding the license agreement, I had agreed to that: https://gist.github.com/justin-calleja/398d9c9c512a7703b786

Thanks again!

Regards, Justin

— Reply to this email directly or view it on GitHub https://github.com/mulesoft/raml-jaxrs-codegen/pull/33#issuecomment-55486131 .

ddossot commented 10 years ago

Apologies, I forgot to publish my acceptance: https://gist.github.com/ddossot/23774fad9a2e3a753360#file-msca-ddossot-md

justin-calleja commented 10 years ago

+1 for hosting on Nexus - that would be :cool:

Justin