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

PR JSR-303 support for Request Parameters + Fix 1244 Allowable values not displayed in Apidocs + 1231 Externalizing Api Descriptions #1254

Closed
wants to merge 114 commits into from

Conversation

jfiala
Copy link
Contributor

@jfiala jfiala commented Apr 9, 2016

Bean Validations API (JSR-303) support for Request Parameters #1227

@jfiala jfiala changed the title PR Bean Validations API (JSR-303) support for Request Parameters #1227 PR Bean Validations API (JSR-303) support for Request Parameters Apr 9, 2016
@jfiala jfiala changed the title PR Bean Validations API (JSR-303) support for Request Parameters PR JSR-303 support for Request Parameters + Fix 1244 Allowable values not displayed in Apidocs Apr 9, 2016
@jfiala
Copy link
Contributor Author

jfiala commented Apr 9, 2016

This PR fixes #1227 and #1244

@jfiala
Copy link
Contributor Author

jfiala commented Apr 16, 2016

Cleaned up sources, now code coverage needs to get improved.

@jfiala
Copy link
Contributor Author

jfiala commented Apr 16, 2016

Added a ApiDescriptionPropertiesReader and associated plugins (Parameter/ExpandedParameter/ModelProperty) to implement #1232

In fact, this should be moved to another module (as it has nothing to do with the validators).
Please suggest where we could move this?

@jfiala jfiala changed the title PR JSR-303 support for Request Parameters + Fix 1244 Allowable values not displayed in Apidocs PR JSR-303 support for Request Parameters + Fix 1244 Allowable values not displayed in Apidocs + 1231 Externalizing Api Descriptions Apr 16, 2016
@jfiala
Copy link
Contributor Author

jfiala commented Jul 2, 2016

@dilipkrish I cleaned up the obvious codacy-bot notes, but what shall we do with "Avoid unused private fields..." regarding the test helper classes? Can you please assist here to get the PR merged eventually...?

@dilipkrish
Copy link
Member

Sure I'll take a look at it. Sorry I din't realize adding codacy support will retroactively reviews PRs.

Also do you mind squashing the merge commits and rebasing against master plus there are merge conflicts that are probably better handled by you.

dilipkrish and others added 7 commits July 2, 2016 12:04
Conflicts:
	springfox-bean-validators/src/main/java/springfox/bean/apidescriptionreaders/plugins/ApiDescriptionPropertiesReader.java
	springfox-bean-validators/src/main/java/springfox/bean/apidescriptionreaders/plugins/ExpandedParameterDescriptionKeysAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/apidescriptionreaders/plugins/ModelPropertyDescriptionKeysAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/apidescriptionreaders/plugins/ParameterDescriptionKeysAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/configuration/BeanValidatorPluginsConfiguration.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ExpandedParameterMinMaxAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ExpandedParameterNotNullAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ExpandedParameterSizeAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/MinMaxAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ModelPropertyNotNullAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ParameterMinMaxAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ParameterNotNullAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/plugins/ParameterSizeAnnotationPlugin.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/util/MinMaxUtil.java
	springfox-bean-validators/src/main/java/springfox/bean/validators/util/SizeUtil.java
	springfox-swagger2/src/main/java/springfox/documentation/swagger2/mappers/EnumMapper.java
@jfiala
Copy link
Contributor Author

jfiala commented Jul 2, 2016

@dilipkrish I rebased and now I get this (both at my forked repo and in the main repo):
gradle eclipsegrad

  • What went wrong:
    A problem occurred evaluating root project 'springfox'.

    Failed to apply plugin [id 'springfox-multi-release']
    Not a valid version. Expecting a version of form <MAJOR.MINOR.PATCH> where e.g. 1.0.0-SNAPSHOT, 1.0.0-1-g10a2eg:

@dilipkrish
Copy link
Member

Looks like its a bad rebase 😖 ... After the rebase you should only be seeing your commits in your pull request. How did you rebase?

Assuming remote origin => springfox/springfox and remote jfiala => jfiala springfox. Is this what you did

git fetch origin master # gets springfox updates
git rebase origin/master #applies your changes on top of springfox HEAD 
git push jfiala master -f #force push to your master so that your pr is updated.

@jfiala
Copy link
Contributor Author

jfiala commented Jul 2, 2016

The PR is in order (after some local cleanups...), I get the error above when doing clean checkout of https://github.com/springfox/springfox/ as well, so it doesn't seem to have to do with my commits...

(and I only changed the springfox-bean-validators + 1 class of springfox-swagger2 - EnumMapper)

@jfiala
Copy link
Contributor Author

jfiala commented Jul 2, 2016

If I view the differences now with jfiala:master, my branch is only 29 commits ahead of the master / 39 files changed? However, I'd like to do a clean build before I create the new PR....

@dilipkrish
Copy link
Member

What do you see when you do a git describe?

@jfiala
Copy link
Contributor Author

jfiala commented Jul 2, 2016

Now it looks good, after I've done gradle eclipse, the subproject springfox-spring-web lost its reference to springfox-schema and also added useless classpathentries "test", e.g. for springfox-schema:

<classpathentry kind="lib" path="C:/projects/springfox/springfox-core/build/classes/test"/>
<classpathentry kind="lib" path="C:/projects/springfox/springfox-core/build/resources/test"/>
...

After cleaning up those I can build again from springfox-master & my master as well.

@dilipkrish
Copy link
Member

Still doesn't look right. Your PR shouldn't have my commits in it 👇
screen shot 2016-07-02 at 3 30 54 pm

@jfiala
Copy link
Contributor Author

jfiala commented Jul 2, 2016

created a new PR which shows the current state (only my / carlosjgp commits...).

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

9 participants