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

Build and test mergely on JDK 16 (on Travis) #9579

Merged
merged 5 commits into from
Apr 22, 2021
Merged

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Apr 19, 2021

No description provided.

@lrytz lrytz requested a review from dwijnand April 19, 2021 10:56
@scala-jenkins scala-jenkins added this to the 2.13.6 milestone Apr 19, 2021
@lrytz lrytz marked this pull request as draft April 19, 2021 11:02
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Just a tiny request, question, and a "let's not do it this way" pushback. 😄

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@som-snytt
Copy link
Contributor

I have a couple of test failures on jdk16... I mentioned it on 3613.

test/files/neg/macro-invalidret-neg.log
test/files/run/t3613-run.log

@SethTisue
Copy link
Member

is the idea here that if we test on 16, we don't also need to test on 11?

@SethTisue SethTisue added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Apr 21, 2021
@lrytz
Copy link
Member Author

lrytz commented Apr 21, 2021

is the idea here that if we test on 16, we don't also need to test on 11?

I'd say that's fine, since we have a community build on 11?

@SethTisue
Copy link
Member

SethTisue commented Apr 21, 2021

I'd say that's fine, since we have a community build on 11?

I don't have a strong feeling either way.

(In the community build, I chose to have both 11 and 16 builds, but I chose that mainly because several important upstream maintainers don't support 15 or 16, so the 11 build tests noticeably more stuff. Here in this repo, that doesn't really apply, so 16 only is probably fine. On the other hand, doing 11 too wouldn't be especially costly. 🤷 )

@som-snytt
Copy link
Contributor

On my lunch break, I am PRing removal of use of ThreadGroup in testkit, which is day job adjacent.

My other innovative contribution is to pronounce PR as purr.

@lrytz lrytz force-pushed the travis-jdk16 branch 2 times, most recently from 68036f4 to 703ea22 Compare April 22, 2021 09:01
@lrytz lrytz marked this pull request as ready for review April 22, 2021 10:12
@lrytz
Copy link
Member Author

lrytz commented Apr 22, 2021

The JDK 16 build went green too: https://travis-ci.com/github/scala/scala/builds/223846952

(I pushed to a scala/scala branch for testing: https://github.com/scala/scala/tree/travis-jdk16)

re-review by @dwijnand

@lrytz
Copy link
Member Author

lrytz commented Apr 22, 2021

(the github page summarizing the travis build incorrectly says ADOPTOPENJDK=8, https://github.com/scala/scala/runs/2408503535. if you look at the build log you see that the env is ADOPTOPENJDK=16, and java -version is correct)

@lrytz lrytz merged commit a936b41 into scala:2.13.x Apr 22, 2021
@SethTisue
Copy link
Member

@som-snytt
Copy link
Contributor

@SethTisue whew, when I saw the notification while waiting at child's piano lesson, I wondered if something broke under -Xsource:3 in the CB.

Thanks for reminding me that I still have to add the ThreadGroup commit to my PR for REPL pretty printing.

SethTisue added a commit to SethTisue/scala that referenced this pull request May 4, 2021
this is a small followup to scala#9579

going warning-free here is a bit tricky, because the method in
question is only deprecated on JDK 9+. so on JDK 8 the old code was
giving a "@nowarn annotation does not suppress any warnings" warning

@lrytz @NthPortal @som-snytt is this best way to handle this?
SethTisue added a commit to SethTisue/scala that referenced this pull request May 4, 2021
this is a small followup to scala#9579

going warning-free here is a bit tricky, because the method in
question is only deprecated on JDK 9+. so on JDK 8 the old code was
giving a "@nowarn annotation does not suppress any warnings" warning

@lrytz @NthPortal @som-snytt is this best way to handle this?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal not resulting in user-visible changes (build changes, tests, internal cleanups)
Projects
None yet
5 participants