-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Enables class-array as parameter to @IncludeCategory and @ExludeCategory #565
Conversation
return new CategoryFilter(new Class<?>[]{categoryType}, null); | ||
} | ||
|
||
public static CategoryFilter include(Class<?>[] categoryType) { |
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 make this Class<?>...
Thanks, @gaffa. This is a great opening. There's a few formatting fixes needed, but I'm more concerned that we might need to adjust the API to maintain compatibility. |
Thank you for detailled feedback. I will adjust the implementation. Also I am pretty sure this is 100% compatible. Do I miss something? |
Done. What do you think? |
private Class<?>[] fExcludedCategories = new Class<?>[]{}; | ||
|
||
public CategoryFilter(Class<?> includedCategory, Class<?> excludedCategory) { | ||
if (includedCategory != null){ |
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.
Style fix: Spaces before braces everywhere, please
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.
Like everywhere? I cannot see any on existing method-calls or constructors. I have already put them at for ( and if (. I will happily include more but I am pretty confused where it misses them.
@dsaff please take a look at @Tibor17 s pull-request (https://github.com/KentBeck/junit/pull/503) before accepting this one. I think he worked stuff out and it might be ready for merge. If you also think so we can close this one as far as I am concerned as he covers it. |
Happy to close it. Thanks for merging 503 in! |
As discussed here: https://github.com/KentBeck/junit/pull/142 and here: https://github.com/KentBeck/junit/pull/503 people are in urgent! need of specifying more than one Class in @ExcludeCategory and @IncludeCategory. It was discussed that there has to be a more sophisticated implementation (ALL/ANY) but I do not really think so. Even if: I would suggest accepting the simple solution as this means a fast benefit for everyone. In the end Selection is a compatible addition and not a different approach.