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
[BEAM-4540] Migrate junit/hamcrest to provided scope. #5604
Conversation
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.
LGTM modulo my comment/question. Please self-merge when fixed/answered.
@@ -37,6 +37,7 @@ dependencies { | |||
shadow library.java.grpc_stub | |||
shadow library.java.grpc_netty | |||
shadow library.java.netty_transport_native_epoll | |||
provided library.java.junit |
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.
Is it a mistake or what is the sense of having it provided and testCompile too? testCompile seems enough.
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.
testCompile only applies to the test source set.
provided applies to the main source set, there are some test utility classes in the main source set that rely on junit classes.
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.
Well my point was about actually the next line where the junit dependency is repeated but as testCompile
which is unneeded given that it is now covered by provided
.
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.
(actually I made a mistake in the comment it was 'provided seems enough'
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.
I see. Now I understand your point.
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username
) to look at it.