-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Added page change #1635
Added page change #1635
Conversation
|
This is ready for review. |
|
This is very great work. Congrats! Please note that Also you should validate that We definitely should add functional test for this. You should check out the |
|
We also did not have the feature that finding the parameter by the name. I think we should not use that. Instead, we should define an order of the parameters when finding the parameter by type is ambiguous. That way you could also use our simple |
|
For finding the method exist or not, do you have any sample case for this? getProcessingEnvironment().getElementUtils().getTypeElement(CanonicalNameConstants.VIEW_PAGER) != null;I'll take a look
For validatorHelper.param.inOrder() //
.type(CanonicalNameConstants.VIEW_PAGER).optional() //
.primitiveOrWrapper(TypeKind.INT).optional() //
.primitiveOrWrapper(TypeKind.FLOAT).optional() //
.primitiveOrWrapper(TypeKind.INT).optional() //
.validate((ExecutableElement) element, validation);What do you think? |
|
For checking a method is avaible, you can use similar code to this. The rest is okay! |
|
You are fast! Thanks for your help. |
|
Now, it ready for reviewing, again. I will go to trip a few days where I am not available for the Internet. Until I will come back from there, could you wait for merging? I will squash commits. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add more test cases (when not all parameters are present)? You can use the same variables because the a new Activity is created for each test.
|
LGTM in general, i added some comments. Thanks! |
|
Thanks! I will fix then as soon as I come back. |
|
Fixed it. It's ready for review, again. |
|
Looks good to me. Please squash the commits and i can merge. |
|
Squashed |
|
Thanks, great PR! Would you mind updating the wiki as well? |
|
No problem! I will let you know as soon as I update wiki. |
see #1568