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

Upgrade to Gradle 4.2.1, remove nebula plugin dependency #5633

Merged
merged 1 commit into from
Oct 3, 2017
Merged

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Oct 2, 2017

This PR upgrades the Gradle runtime to 4.2 (which supports Java 9) and gets rid of the rxjava-nebula plugin. Its two features, publishing a snapshot and publishing a release has been manually implemented in build.gradle based on some [online examples] and RxAndroid's variant.

The snapshot publication works and to test the release, we may have to burn a couple of version tags from 2.1.x.

The PR also renamed the perf directory into jmh because the replacement JMH plugin expects those files there.

@akarnokd akarnokd added this to the 2.2 milestone Oct 2, 2017
@codecov
Copy link

codecov bot commented Oct 2, 2017

Codecov Report

Merging #5633 into 2.x will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5633      +/-   ##
============================================
+ Coverage     96.16%   96.29%   +0.13%     
- Complexity     5843     5852       +9     
============================================
  Files           634      634              
  Lines         41539    41539              
  Branches       5752     5752              
============================================
+ Hits          39946    40002      +56     
+ Misses          650      608      -42     
+ Partials        943      929      -14
Impacted Files Coverage Δ Complexity Δ
...vex/internal/subscribers/QueueDrainSubscriber.java 50% <0%> (-1.57%) 15% <0%> (-1%)
...ternal/operators/completable/CompletableMerge.java 96.42% <0%> (-1.2%) 2% <0%> (ø)
...ivex/internal/operators/maybe/MaybeMergeArray.java 96.62% <0%> (-1.13%) 6% <0%> (ø)
...ernal/operators/flowable/FlowableFromIterable.java 95.18% <0%> (-1.07%) 5% <0%> (ø)
.../operators/observable/ObservableFlatMapSingle.java 94.77% <0%> (-0.75%) 2% <0%> (ø)
...l/operators/observable/ObservableFlatMapMaybe.java 88.23% <0%> (-0.66%) 2% <0%> (ø)
...al/operators/observable/ObservableWindowTimed.java 90.73% <0%> (-0.28%) 4% <0%> (ø)
...vex/internal/operators/parallel/ParallelRunOn.java 97.46% <0%> (+0.5%) 6% <0%> (ø) ⬇️
...io/reactivex/subscribers/SerializedSubscriber.java 95.45% <0%> (+1.13%) 26% <0%> (+1%) ⬆️
...java/io/reactivex/processors/UnicastProcessor.java 95.9% <0%> (+1.16%) 64% <0%> (+1%) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea4d43a...f41b069. Read the comment docs.

Copy link
Contributor

@artem-zinnatullin artem-zinnatullin 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, I'd like to check Jar Manifest config more closely later when I have time if you don't mind

build.gradle Outdated
classpath 'gradle.plugin.nl.javadude.gradle.plugins:license-gradle-plugin:0.13.1'
classpath "me.champeau.gradle:jmh-gradle-plugin:0.4.4"
classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.7.3'
classpath "org.jfrog.buildinfo:build-info-extractor-gradle:latest.release"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd strongly recommend to replace latest.release with particular version like 4.5.2, otherwise build won't be reproducible

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but these are snapshot/release builds which are by definition non-reproducible as there can be only one version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, non-reproducible also means that RxJava release might fail unpredictably because this plugin was updated even if no one changed anything Gradle-related in RxJava itself…

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay.

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-2.14-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.2-bin.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please run something like ./gradlew --version on this branch?

It should update gradle/wrapper/gradle-wrapper.jar and gradlew and you need to commit them

Copy link
Member Author

Choose a reason for hiding this comment

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

I did exactly that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like 4.2.1 came out just now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well yes, but it still should have updated wrapper jar and bash script hm

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

@akarnokd akarnokd changed the title Upgrade to Gradle 4.2, remove nebula plugin dependency Upgrade to Gradle 4.2.1, remove nebula plugin dependency Oct 2, 2017
build.gradle Outdated
apply plugin: 'nebula.rxjava-project'
apply plugin: 'maven'
apply plugin: 'osgi'
apply plugin: "me.champeau.gradle.jmh"
Copy link
Contributor

Choose a reason for hiding this comment

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

super nit: can you please use either single or double quotes for consistency?

(Separate PR was always overkill for that, but since you're modifying the files it feels like a great opportunity to finally do that)

build.gradle Outdated
instruction 'Bundle-Vendor', 'RxJava Contributors'
instruction 'Bundle-DocURL', 'https://github.com/ReactiveX/RxJava'
instruction 'Import-Package', '!org.junit,!junit.framework,!org.mockito.*,*'
instruction 'Eclipse-ExtensibleAPI', 'true'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this in manifest file of 2.1.4, do we need it?

build.gradle Outdated
name = 'rxjava'
instruction 'Bundle-Vendor', 'RxJava Contributors'
instruction 'Bundle-DocURL', 'https://github.com/ReactiveX/RxJava'
instruction 'Import-Package', '!org.junit,!junit.framework,!org.mockito.*,*'
Copy link
Contributor

Choose a reason for hiding this comment

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

What about testNG?

As an alternative, you can explicitly declare only org.reactivestreams;version="[1.0,2)".
Or we can figure out how to dynamically collect compile dependencies to put them here

Copy link
Member Author

Choose a reason for hiding this comment

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

I think these are needed by the OSGi plugin MANIFEST.MF. I don't know how relevant this is.

Copy link
Collaborator

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

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

Let's then also remove the findbugs & pmd apply plugins calls that are commented out?

@akarnokd
Copy link
Member Author

akarnokd commented Oct 3, 2017

Removed findbugs and pmd references. Squashed.

@akarnokd
Copy link
Member Author

akarnokd commented Oct 3, 2017

Let's merge it then. Note the artifactory repo seems to deduplicate files. I referenced the snapshot in a separate project and it worked.

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