Skip to content

Fix errorprone to version 2.3.1 - #6657

Merged
akedin merged 1 commit into
apache:masterfrom
akedin:errorprone-fix-version
Oct 11, 2018
Merged

Fix errorprone to version 2.3.1#6657
akedin merged 1 commit into
apache:masterfrom
akedin:errorprone-fix-version

Conversation

@akedin

@akedin akedin commented Oct 11, 2018

Copy link
Copy Markdown
Contributor

Please add a meaningful description for your change here


Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

@akedin

akedin commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

R: @swegner @youngoli @kennknowles

@youngoli youngoli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have one question, but I'm approving this because I've confirmed it works and we should merge in a fix ASAP to unblock people.

// Enable errorprone static analysis
project.apply plugin: 'net.ltgt.errorprone'

project.configurations.errorprone { resolutionStrategy.force 'com.google.errorprone:error_prone_core:2.3.1' }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the description for the plugin (https://github.com/tbroyer/gradle-errorprone-plugin) it says to simply include errorprone as a dependency, like so:

dependencies {
    errorprone("com.google.errorprone:error_prone_core:$errorproneVersion")
}

Can we do it that way? Or are we forced to do it this way because the errorprone plugin is an older version than the one in the link?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It needs to be after the plugin is applied

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's because of how we apply the plugin and other plugins in our own plugin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, the fact that we cannot specify the dependency is because the old plugin we use doesn't work that way

@akedin

akedin commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

Run Website_Stage_GCS PreCommit

@kennknowles

Copy link
Copy Markdown
Member

Seems website staging is perma-red. Definitely merge if Java goes green.

@akedin

akedin commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

Run Website_Stage_GCS PreCommit

@swegner

swegner commented Oct 11, 2018

Copy link
Copy Markdown
Contributor

LGTM

No need to block on website pre-commit. They should be fixed here, but it might require merging those changes into this PR first.

@akedin

akedin commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

My understanding was that running precommits would rebase on top of master, we'll find out

@akedin

akedin commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

Nope it didn't :)

@akedin
akedin merged commit baf5d3b into apache:master Oct 11, 2018
@akedin akedin mentioned this pull request Oct 11, 2018
2 tasks
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.

4 participants