Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Conversation

@yDelouis
Copy link
Contributor

Fixes #1443.

Copy link
Member

Choose a reason for hiding this comment

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

Missing test scope!

Copy link
Member

Choose a reason for hiding this comment

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

Moreover, we use Mockito in other modules as well. So you can pull it to the root POM in dependencyManagement, just like we do with JUnit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@WonderCsabo
Copy link
Member

The test suite is far from full coverage, but it is a good start. Thanks for this PR, we really needed this fix, and we can move on.

It seems Travis is down, so that is why it did not tell you have lots of Checkstyle errors in the test class and a missing licence header. Please fix this, as well as my other comments. After it, the PR can be merged.

@WonderCsabo
Copy link
Member

We have a problem:

ItemClicksHandledActivity.java:[116,9] org.androidannotations.annotations.ItemSelect can only have the following parameters: [ [ boolean or java.lang.Boolean ], [ extending java.lang.Object (optional) ],  ] in the order above
ItemClicksHandledActivity.java:[131,9] org.androidannotations.annotations.ItemSelect can only have the following parameters: [ [ boolean or java.lang.Boolean ], [ extending java.lang.Object (optional) ],  ] in the order above
ItemClicksHandledActivity.java:[142,9] org.androidannotations.annotations.ItemSelect can only have the following parameters: [ [ boolean or java.lang.Boolean ], [ extending java.lang.Object (optional) ],  ] in the order above

The problem here that ItemSelect really accepts any type (any reference type or primitive type), except that if the second parameter is primitive int, than it is treated as a position, otherwise as the element.
But anyType() does not accept any types, only any classes. We should rename it to anyClass() or such, then introduce a new anyType() validator which indeed accepts any type (primitive or class).

@yDelouis yDelouis force-pushed the 1443_fixInOrderParamValidation branch from 9f3caee to 096b087 Compare September 20, 2015 16:31
@yDelouis
Copy link
Contributor Author

I addressed your comments.
I hope @dodgex is okay for setting him as author of the last commit since I copied-pasted his gist.

@dodgex
Copy link
Member

dodgex commented Sep 20, 2015

@yDelouis sure i'm okay with that. Thanks :)

@WonderCsabo
Copy link
Member

@yDelouis i just saw the getWrapperType() method, and it is half-done, not sure why. Can you fix that?

@WonderCsabo
Copy link
Member

@yDelouis also please rebase this so the build can go green.

@WonderCsabo
Copy link
Member

@yDelouis any update on this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants