Skip to content

Conversation

@eolivelli
Copy link
Contributor

Motivation

JDK14 introduced java.lang.Record, that is imported by default as "Record".
The build fails in some .java files due to the ambiguity between org.apache.pulsar.functions.api.Record and java.lang.Record.

Modifications

Use explicit imports instead of star import, only in the Java files that break the build.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Enrico Olivelli added 3 commits October 26, 2020 08:18
- update Apache Parent Maven Pom to latest (upgrade plugins)
- use explicit import for Record class (prevent clash with java.lang.Record)
@eolivelli
Copy link
Contributor Author

I am trying to split the patch into smaller pieces because in #8360 integration tests and other CI jobs does not pass, probably due to the upgrade of the Maven Assembly plugin.
I am going to send a separate patch for the Maven Assembly Plugin stuff.

@merlimat you already approved #8360, hopefully here the tests will pass without problem (otherwise we have serious issues on CI), PTAL and merge if you can. thanks in advance

cc @codelipenghui @rdhabalia @jiazhai

@eolivelli
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

2 similar comments
@eolivelli
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@eolivelli
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@eolivelli
Copy link
Contributor Author

I am seeing the same failure many times
#8374
I will address the issue in a separate patch

@eolivelli
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

2 similar comments
@eolivelli
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@eolivelli
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@eolivelli
Copy link
Contributor Author

I have sent a separate fix for the flaky test
#8379

@merlimat merlimat added this to the 2.7.0 milestone Oct 26, 2020
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@eolivelli the change looks good! But I think we should also consider enabling checkstyle across module to enforce consistent styles. There are some modules already running checkstyle. Are you interested in contributing code changes to enable them?

@eolivelli
Copy link
Contributor Author

I had another change about upgrading checkstyle to latest version.
It would be a quite huge patch (and boring work) because they changed the ImportOrder rule

@merlimat merlimat merged commit c6560ee into apache:master Oct 26, 2020
@eolivelli eolivelli deleted the fix/build-jdk15-part1 branch October 27, 2020 14:14
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Nov 13, 2020
…n order to be able to build Pulsar on JDK14+ (apache#8370)

* Allow to build Pulsar on JDK14
- update Apache Parent Maven Pom to latest (upgrade plugins)
- use explicit import for Record class (prevent clash with java.lang.Record)

* xx

* revert pom changes

Co-authored-by: Enrico Olivelli <enrico.olivelli@diennea.com>
flowchartsman pushed a commit to flowchartsman/pulsar that referenced this pull request Nov 17, 2020
…n order to be able to build Pulsar on JDK14+ (apache#8370)

* Allow to build Pulsar on JDK14
- update Apache Parent Maven Pom to latest (upgrade plugins)
- use explicit import for Record class (prevent clash with java.lang.Record)

* xx

* revert pom changes

Co-authored-by: Enrico Olivelli <enrico.olivelli@diennea.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants