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

Update samples for java/jersey2-java6 (and fix artifact ID) #5118

Merged
merged 3 commits into from
Mar 23, 2017

Conversation

ePaul
Copy link
Contributor

@ePaul ePaul commented Mar 19, 2017

PR checklist

  • Read the contribution guidelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change.
    → bin/java-petstore-jersey2-java6.sh
  • Filed the PR against the correct branch: master for non-breaking changes.

Description of the PR

This has no generator code changes, just updates some samples so later code changes see the actual effect on the samples. I also updated the sample generation script to use -artifact-id swagger-petstore-jersey2-java6 so the generation doesn't destroy the changed artifact name in the generated pom.xml. (There doesn't seem to be a windows script for this.)

Copy link
Contributor Author

@ePaul ePaul left a comment

Choose a reason for hiding this comment

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

Pointing out where the changes come from.

# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From #4197.

@@ -0,0 +1,15 @@

# Capitalization
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added by #4458.

@@ -0,0 +1,10 @@

# ClassModel
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added by #4237.

@@ -15,6 +15,8 @@ Method | HTTP request | Description

To test \"client\" model

To test \"client\" model
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This duplicated line is here because the summary and description is the same. With 9051848 (I don't see a PR for this) the key was fixed from descriptions (which was ignored) to description.

@@ -0,0 +1,14 @@

# OuterEnum
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added by #4164 for #4160/#4088.

@@ -53,7 +53,7 @@ public Client testClientModel(Client body) throws ApiException {
}

// create path and map variables
String localVarPath = "/fake".replaceAll("\\{format\\}","json");
String localVarPath = "/fake";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes came from #4987.

@@ -124,7 +124,7 @@ public void deletePet(Long petId, String apiKey) throws ApiException {
* Finds Pets by status
* Multiple status values can be provided with comma separated strings
* @param status Status values that need to be considered for filter (required)
* @return List<Pet>
* @return List&lt;Pet&gt;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#4616 changed {{{returnType}}} to {{returnType}} here.

@JsonSubTypes({
@JsonSubTypes.Type(value = Dog.class, name = "Dog"),
@JsonSubTypes.Type(value = Cat.class, name = "Cat"),
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to come from #4085.

@@ -59,7 +47,7 @@ public AdditionalPropertiesClass putMapPropertyItem(String key, String mapProper
* Get mapProperty
* @return mapProperty
**/
@ApiModelProperty(example = "null", value = "")
@ApiModelProperty(value = "")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like previously we got a string "null" instead of a real null for the example, which confused the null-check.
I didn't figure out which commit/PR fixed this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ePaul I remember Tony Tam filed a PR to fix it. If you need the number, I can dig it out for you.

@@ -4,7 +4,7 @@
## Properties
Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
**uuid** | **String** | | [optional]
**uuid** | [**UUID**](UUID.md) | | [optional]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That looks wrong. (The format: uuid was in the YAML from the beginning. Possibly #4739 changing to use java.util.UUID instead of String added the link here?

Related: #5026. #5027 might possibly fix this too.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ePaul there's a discussion about updating isPrimitiveType to include UUID as well. IMO, isPrimitiveType should include UUID.

(the issue was caused by the change that maps uuid as UUID instead of String in Java)

@ePaul ePaul changed the title Update samples for java/jersey2-java6. Update samples for java/jersey2-java6 (and fix artifact ID) Mar 19, 2017
@wing328
Copy link
Contributor

wing328 commented Mar 23, 2017

@ePaul thanks for updating the sample. I'll update ./bin/java-petstore-all.sh to include bin/java-petstore-jersey2-java6.sh as well so that the Petstore client (jersey2, java6) won't be outdated.

@wing328 wing328 merged commit 9dc8cf3 into swagger-api:master Mar 23, 2017
@ePaul ePaul deleted the update-samples-jersey2-java6 branch March 23, 2017 09:00
spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
…api#5118)

* Update samples for java/jersey2-java6.

* Let the sample generation script set the right name for jersey2-java6.

This is the equivalent change to swagger-api#5095, for jersey2-java6.

* Update samples for Java/Jersey2-java6.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants