HV-1119 Verify that annotation parameters on method/constructor parameters are valid #538
Conversation
There was a problem hiding this comment.
Looks like your formatter is doing weird things with annotations. The initial formatting is what our Eclipse formatter does (I fixed it when merging your previous PR).
There was a problem hiding this comment.
Same here, it should be: @Digits(integer = -1, fraction = 3).
I would have fixed it when merging but as you are a regular contributor, we might as well have your formatters right ;). We provide IDE configuration here: https://github.com/hibernate/hibernate-ide-codestyles . I know for sure the Eclipse ones are OK but if you use Intellij and there are problems with the configuration file we provide, we might as well fix them! Thanks for your feedback on this!
There was a problem hiding this comment.
Thanks for the link on formatters - it worked just great! Also importing of formatter in Intellij can be done through the UI rather than copying files :)
One day I'll create a PR and you wouldn't need to clean up anything after me :)
There was a problem hiding this comment.
We're nearly there! We have a space after { in the annotations and it looks like the Intellij formatter removes it.
It should be:
@Digits.List({ @Digits(integer = -5, fraction = -3), @Digits(integer = 5, fraction = -3) })If it's an issue with the formatter we provide, could you fix it and open a PR for it?
Once this one is solved, I think we won't have any formatting issue left! :)
There was a problem hiding this comment.
should be correct now :) I'll open a PR for formatter with corresponding update
| * <p> | ||
| * Checks whether the given annotations are correctly specified at the given | ||
| * method parameter. The following checks are performed: | ||
| * </p> |
There was a problem hiding this comment.
OK, so everything looks good apart from this comment here. With this patch, we actually check the constraints validity on parameters. I think the comment is meant for the other patch. Could you fix it? Then I'll merge this one. Thanks!
There was a problem hiding this comment.
Good catch :) this was copied over from HV-840. Thanks ! I've made a change.
|
Merged! Thanks! |
2 participants
added checks for parameters and corresponding test cases
This one is a part from changes made in HV-840 to complete what was started in HV-270