Skip to content

Conversation

@roni-frantchi
Copy link
Author

@fehguy ?... anyone...?
It's been almost 6 months now...
Any reason not to merge?.. Any changes that I should make?..

@webron
Copy link
Contributor

webron commented Sep 28, 2016

Hi @roni-frantchi - the biggest reason is time :) We can possibly review this in about two weeks (definitely not this week), but I can't guarantee that. It looks like this PR needs to be resynced with the code before it can be merged.

@webron
Copy link
Contributor

webron commented Sep 28, 2016

Oh, and Shana Tova.

@roni-frantchi
Copy link
Author

@webron will get right on resolving those pesky conflicts and push for reviewal

Shana Tova :)

…into 'master' of https://github.com/sapienstech/swagger-core

# Conflicts:
#	modules/swagger-annotations/pom.xml
#	modules/swagger-core/pom.xml
#	modules/swagger-core/src/test/resources/Cat.json
#	modules/swagger-hibernate-validations/pom.xml
#	modules/swagger-jaxrs/pom.xml
#	modules/swagger-jersey-jaxrs/pom.xml
#	modules/swagger-jersey2-jaxrs/pom.xml
#	modules/swagger-models/pom.xml
#	modules/swagger-mule/pom.xml
#	modules/swagger-servlet/pom.xml
#	pom.xml
@roni-frantchi
Copy link
Author

@webron @frantuma @fehguy any news on this one?...

@fehguy
Copy link
Contributor

fehguy commented Dec 19, 2016

I'm sorry for this taking so long, but this introduces attributes which are illegal in the spec (parent) so we can't merge it.

@fehguy fehguy closed this Dec 19, 2016
@roni-frantchi
Copy link
Author

@fehguy wait, what?
First, wasn't 'parent' an attribute that was already available?
Second, isn't the spec extendible so that one can add further metadata?
Third, how about a suggestion of what would be a valid approach?

@fehguy
Copy link
Contributor

fehguy commented Dec 20, 2016

There is a difference between having a property in the java pojo and in the JSON/YAML description. This:

https://github.com/sapienstech/swagger-core/blob/2e5df42c7053660c8fc34e4e6cb128a9d4e3661b/modules/swagger-core/src/test/resources/Cat.json#L15

Is not valid in the specification:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md

And can't be there. We can have private fields in the pojos, but they cannot live in the JSON generated.

@roni-frantchi
Copy link
Author

roni-frantchi commented Dec 20, 2016

There is a difference between having a property in the java pojo and in the JSON/YAML description.

@fehguy that's understood. Looking for recommendation of what fixes I may apply here that would make the approach valid.

@roni-frantchi
Copy link
Author

roni-frantchi commented Dec 20, 2016

@fehguy the spec says:

While the Swagger Specification tries to accommodate most use cases, additional data can be added to extend the specification at certain points.
The extensions properties are always prefixed by "x-" and can have any valid JSON format value.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#specification-extensions

So, if I add that prefix, meaning parent would become x-parent, that would become a valid approach, right?
Would you like me to open another PR with that change in place?

roni-frantchi added a commit to sapienstech/swagger-core that referenced this pull request Dec 20, 2016
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.

4 participants