Skip to content
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

Reclaim scala package #527

Merged
merged 4 commits into from
Jun 20, 2018
Merged

Reclaim scala package #527

merged 4 commits into from
Jun 20, 2018

Conversation

softprops
Copy link
Contributor

this relates to #526 ( specifically this comment ). The base package of the entire test hardness is scala which just happens to be the base package of scala std lib. This prevents the ability do refer to things like scala.Option within mixed java sources.

@softprops
Copy link
Contributor Author

note this looks like a lot ( and it is ) but its exclusively renames of the test harness packaging from scala to scalarules.

deps = ["MixJavaScalaLib"],
)

#Mix java scala (srcjar), much like above only the java is provided via srcjar
scala_library(
name = "MixJavaScalaSrcjarLib",
srcs = glob([
"src/main/scala/scala/test/mix_java_scala/*.scala",
"src/main/scala/scalarules/test/mix_java_scala/*.scala",
# srcjar created with `jar -cfM Baz.srcjar Baz.java`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these comments were well with the extra few seconds it took to add them. I dealt with a number of failing tests related to jars that I had to recreate using the commentary in these rules.

@softprops
Copy link
Contributor Author

Looks like a failing scala_specs2_exception_in_initializer_without_filter test. Ill look into that.

@softprops
Copy link
Contributor Author

@johnynek or others it looks like the builds is passing expect for one item on the build matrix which I can't tell if its related to my changes here

@johnynek
Copy link
Member

thanks @softprops it looks like a transient download issue. restarting.

@johnynek johnynek merged commit 93a2ad9 into bazelbuild:master Jun 20, 2018
@johnynek
Copy link
Member

thanks @softprops !

@ittaiz
Copy link
Member

ittaiz commented Jun 20, 2018

thanks!

@softprops
Copy link
Contributor Author

Np. I think I can followup on the mixed compilation thing with a failing test case. I'll likely need to get up to speed on what's changed since I last understood the code base. In any case this exercise gave me a greater appreciation for the amount of testing in these rules. One thing may suggest is tips for contributors on getting faster turn around cycles. Rename / retest cycles for this particular pr were massive!

@johnynek
Copy link
Member

yeah, that's good feedback @softprops. A better doc on tips for contributing would be very useful.

Also, if we could port more of the tests to be in bazel so that running them is as easy as bazel test test/... Getting negative tests inside bazel seems at least like a recursive call to bazel, something that has always made me nervous, but maybe is okay. There was some work on a bazel testing framework, and we even have a PR: #503 -- if we can expand on that, it might decrease iteration time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants