Skip to content

KAFKA-7524: Recommend Scala 2.12 and use it for development#5530

Merged
ijuma merged 2 commits intoapache:trunkfrom
ijuma:build-scala-2.12-default
Oct 28, 2018
Merged

KAFKA-7524: Recommend Scala 2.12 and use it for development#5530
ijuma merged 2 commits intoapache:trunkfrom
ijuma:build-scala-2.12-default

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Aug 18, 2018

Scala 2.12 has better support for newer Java versions and includes additional
compiler warnings that are helpful during development. In addition, Scala 2.11
hasn't been supported by the Scala community for a long time, the soon to be
released Spark 2.4.0 will finally support Scala 2.12 (this was the main reason
preventing many from upgrading to Scala 2.12) and Scala 2.13 is at the RC stage.
It's time to start recommending the Scala 2.12 build as we prepare support for
Scala 2.13 and start thinking about removing support for Scala 2.11.

In the meantime, Jenkins will continue to build all supported Scala versions (including
Scala 2.11) so the PR and trunk jobs will fail if people accidentally use methods
introduced in Scala 2.12.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@ijuma ijuma requested review from ewencp and hachikuji August 18, 2018 14:49
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Aug 18, 2018

@ewencp @hachikuji What do you think?

@ijuma ijuma force-pushed the build-scala-2.12-default branch from ee347e4 to dbb5102 Compare August 18, 2018 16:26
Copy link
Copy Markdown
Contributor

@ewencp ewencp left a comment

Choose a reason for hiding this comment

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

Given this is really about development and we now require jdk8 anyway, this generally seems fine to me. Just a few comments.

First, we should probably define what we're doing here -- are we changing what we consider the default version generally or just during development. The changes to kafka-run-class.sh indicate the former.

  • Assuming complete default change, upgrade notes need to note this change because if we're still publishing 2.11 artifacts, the natural thing to do to upgrade versions is to just provide new 2.11 artifacts and roll, but this would break any deployment where SCALA_VERSION is not explicitly specified. Hate to say it, but changing those bits is possibly KIP-worthy (but an easy one).

  • In either case, there's a streams reference to 2.11 in docs/api.html. Not sure what the goal is for that -- if it's supposed to match the default here then we should change it. Similar deal in docs/streams/developer-guide/write-streams.html . If we are changing the project-wide default, any references like this should be changed, though grepping only turned up those two unless I wasn't thorough.

  • Quickstart also references 2.11 version. Still fine as we release it, but if we want to encourage 2.12 as default, that should change. Admittedly this matters a lot less than the version specified when using libraries.

Also, unrelated, but since I noticed it when checking for 2.11 references, the upgrade tests seem like they may need an update (which we should do after every release) - vagrant/base.sh and tests/docker/Dockerfile don't have a step for downloading and extracting 2.0.0, which seems like a gap now.

@ijuma ijuma force-pushed the build-scala-2.12-default branch 3 times, most recently from e9dc586 to a3ae4a6 Compare October 14, 2018 06:47
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Oct 14, 2018

Thanks for the feedback @ewencp. The reason why I changed kafka-run-class is so that the scripts work during development.

the natural thing to do to upgrade versions is to just provide new 2.11 artifacts and roll, but this would break any deployment where SCALA_VERSION is not explicitly specified

Actually it will not have any impact on binary releases since SCALA_VERSION only matters running code built from source. So, I don't think there's compatibility impact here and hence why I didn't think an upgrade note or KIP was needed. Am I missing anything?

Copy link
Copy Markdown
Contributor

@ewencp ewencp left a comment

Choose a reason for hiding this comment

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

@ijuma Nope, I was missing it -- have to look through the rest of kafka-run-class for it to be clear it's only for dev (and even then you have to know the dev/release layouts...)

LGTM

@ijuma ijuma force-pushed the build-scala-2.12-default branch from a3ae4a6 to cdc2bd1 Compare October 21, 2018 02:21
@ijuma ijuma changed the title MINOR: Build with Scala 2.12 by default KAFKA-7524: Recommend Scala 2.12 and use it for development Oct 21, 2018
ijuma added 2 commits October 27, 2018 09:36
Scala 2.12 has useful compiler warnings
that are helpful during development. Jenkins
will continue to build all supported Scala
versions (including Scala 2.11) so the PR
builder will fail if people accidentally
use methods introduced in Scala 2.12.
@ijuma ijuma force-pushed the build-scala-2.12-default branch from cdc2bd1 to 1bb4848 Compare October 27, 2018 16:36
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Oct 27, 2018

@ewencp I added one commit after your review, does it look OK?

Copy link
Copy Markdown
Contributor

@ewencp ewencp left a comment

Choose a reason for hiding this comment

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

@ijuma yeah, LGTM

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Oct 28, 2018

JDK 11 failure is unrelated. Merged to trunk.

@ijuma ijuma merged commit b4722cc into apache:trunk Oct 28, 2018
@ijuma ijuma deleted the build-scala-2.12-default branch October 28, 2018 18:31
ijuma added a commit to confluentinc/common that referenced this pull request Oct 29, 2018
Related public PRs:

* apache/kafka#5530
* confluentinc/ksql#2074
* confluentinc/support-metrics-client#54

Tested with muckrake that all projects compile fine if all the PRs are merged.
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
)

Scala 2.12 has better support for newer Java versions and includes additional
compiler warnings that are helpful during development. In addition, Scala 2.11
hasn't been supported by the Scala community for a long time, the soon to be
released Spark 2.4.0 will finally support Scala 2.12 (this was the main reason
preventing many from upgrading to Scala 2.12) and Scala 2.13 is at the RC stage.
It's time to start recommending the Scala 2.12 build as we prepare support for
Scala 2.13 and start thinking about removing support for Scala 2.11.

In the meantime, Jenkins will continue to build all supported Scala versions (including
Scala 2.11) so the PR and trunk jobs will fail if people accidentally use methods
introduced in Scala 2.12.

Reviewers: Ewen Cheslack-Postava <me@ewencp.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants