[BEAM-254] Add apache-rat-plugin execution and fix files with missing… by jbonofre · Pull Request #297 · apache/beam

jbonofre · 2016-05-06T07:06:40Z

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

Make sure the PR title is formatted like:
[BEAM-<Jira issue #>] Description of pull request
Make sure tests pass via mvn clean verify. (Even better, enable
Travis-CI on your fork and ensure the whole test matrix passes).
Replace <Jira issue #> in the title with the actual Jira issue
number, if there is one.
If this contribution is large, please file an Apache
Individual Contributor License Agreement.

Add apache-rat-plugin execution and fix files with missing license header.

jbonofre · 2016-05-06T07:07:19Z

swegner · 2016-05-06T16:38:54Z

@@ -1,3 +1,22 @@
+<!--


Do these comments show up in the github pull request description field?

No, it's hidden in github (markdown).

swegner · 2016-05-06T16:56:04Z

According to this example:

In larger projects, the plugin may take some time to run. In such cases, it may be desirable not to run the plugin with every build, but only in important cases like a release build

Should we have this in a separate profile used by pre/post-commit tests? /cc @kennknowles who was been doing work to optimize build.

jbonofre · 2016-05-06T17:32:52Z

Yes, I can create a dedicated profile for rat. But, I think it makes sense to enable it during the maven-release-plugin execution.

kennknowles · 2016-05-06T17:40:03Z

In #286 I created a release profile. We could put it into that. Or having a separate profile seems fine too, and we can mix-and-match what things we enable. Can you have a release profile that automatically activates the rat profile? I haven't gotten that deep into it.

jbonofre · 2016-05-06T17:46:52Z

Sure, it makes sense.

jbonofre · 2016-05-11T00:35:54Z

Rebased and updated according to the comments.

jbonofre · 2016-05-20T08:54:30Z

Any comment regarding the latest update ?

dhalperi · 2016-05-20T15:44:13Z


 The functions are generic so it supports join of any types supported by
-Dataflow. Input to the join functions are PCollections of Key/Values. Both the
+Beam. Input to the join functions are PCollections of Key/Values. Both the


jbonofre · 2016-05-21T06:32:12Z

I'm updating according to @dhalperi comments. Thanks Dan !

jbonofre · 2016-05-23T16:45:43Z

Rebased and updated.

davorbonaci · 2016-05-23T16:48:48Z

Triage: 0.1.0-incubating, release blocking.

davorbonaci · 2016-05-23T16:49:23Z

As per in-person discussion, let's remove header enforcement from checkstyle, and run it exclusively through rat on all files, including java.

jbonofre · 2016-05-23T18:46:41Z

Updated to use rat for java license header check (and disable checkstyle for that).

davorbonaci · 2016-05-23T19:51:51Z

+ <excludeSubProjects>false</excludeSubProjects>
+ <useDefaultExcludes>true</useDefaultExcludes>
+ <excludes>
+ <!-- exclude target -->


replace all comments with one that says to keep in sync with .gitignore?

actually, this should be super-set of gitignore.

jbonofre · 2016-05-23T20:02:33Z

Updated.

jbonofre · 2016-05-23T20:06:48Z

Updated fixing test_wordcount.sh comment.

jbonofre · 2016-05-24T06:06:38Z

Rat check fails on Jenkins whereas it works on my local copy. I'm checking the Jenkins workspace to identify invalid files.

davorbonaci · 2016-05-24T11:42:44Z

@jbonofre, let me know when you get a chance to fix this -- we'd like to take this asap.

jbonofre · 2016-05-24T11:45:39Z

I'm on it, Thanks !

jbonofre · 2016-05-24T12:45:25Z

Rebased, and testing rat configuration.

jbonofre · 2016-05-24T12:57:02Z

Still failing with: Caused by: org.apache.rat.mp.RatCheckException: Too many files with unapproved license: 3796 See RAT report in: /home/jenkins/jenkins-slave/workspace/beam_PreCommit_MavenVerify/target/parent-0.1.0-incubating-SNAPSHOT.rat

I'm checking the content of the rat file.

jbonofre · 2016-05-24T13:14:05Z

Updated RAT configuration for Jenkins (as it uses "local" .repository).

jbonofre · 2016-05-24T14:13:19Z

Same issue with Kafka on Jenkins, it's not related to the PR.

dhalperi · 2016-05-24T15:20:50Z

Do you need to rebase on master? I pushed the fix to Kafka yesterday.
On Tue, May 24, 2016 at 07:13 Jean-Baptiste Onofré notifications@github.com
wrote:

Same issue with Kafka on Jenkins, it's not related to the PR.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#297 (comment)

jbonofre · 2016-05-24T15:28:00Z

Sorry, the issue is not on Kafka: it's the javadoc on Flink runner.

jbonofre · 2016-05-24T15:32:30Z

By the way, I reproduce the issue locally with: mvn clean install -Prelease

dhalperi · 2016-05-24T16:35:11Z

Kenn is looking at the build break. Thanks for the repro jb.

On Tue, May 24, 2016 at 08:32 Jean-Baptiste Onofré notifications@github.com
wrote:

By the way, I reproduce the issue locally with: mvn clean install -Prelease
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#297 (comment)

kennknowles · 2016-05-24T17:12:06Z

There are multiple issues:

Javadoc should not (necessarily) be generated in precommit. Actually it is fine to re-add it, as long as we don't burn cycles building the jar.
We intend to disable javadoc for broken builds.

I actually think the easiest way to solve 2 would be to roll forwards by fixing the errors and silencing the warnings. There are very few actual errors.

For now, I will just try to sort out why things are building when they should not be (both 1 & 2).

dhalperi · 2016-05-24T18:15:41Z

On Tue, May 24, 2016 at 10:19 AM, Kenn Knowles notifications@github.com
wrote:

There are multiple issues:
Javadoc should not (necessarily) be generated in precommit.
Actually it is fine to re-add it, as long as we don't burn cycles building
the jar.
If we're going to block release on Javadoc compiling successfully, I think
we do want Javadoc generation to be tested in pre-commit.
We intend to disable javadoc for broken builds.
I actually think the easiest way to solve 2 would be to roll forwards by
fixing the errors and silencing the warnings. There are very few actual
errors.
For now, I will just try to sort out why things are building when they
should not be (both 1 & 2).
SGTM
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#297 (comment)

jbonofre · 2016-05-24T18:42:32Z

It makes sense. On the other hand, I fixed javadoc issue in PR #382.

FYI, the java8 examples javadoc has to be fixed too (I'm working on it).

kennknowles · 2016-05-24T19:58:21Z

On pre vs post testing, I agree that ideally we'd test everything that matters for release in presubmit, but our build is already problematically slow. I think javadoc:javadoc is pretty fast, to catch errors, while we can do some maven configuration to elide the slower javadoc jar.

davorbonaci · 2016-05-31T17:18:42Z

Looks like this needs a rebase to re-run the tests, and then I can merge.

LGTM

… license header

jbonofre · 2016-05-31T19:30:53Z

Jenkins failed due to timeout (not related directly to the PR):
[INFO] I/O exception (java.net.SocketException) caught when processing request to {s}->https://repo.maven.apache.org:443: Connection timed out
[INFO] I/O exception (java.net.SocketException) caught when processing request to {s}->https://repo.maven.apache.org:443: Connection timed out
[INFO] Retrying request to {s}->https://repo.maven.apache.org:443
[INFO] Retrying request to {s}->https://repo.maven.apache.org:443
Build timed out (after 100 minutes). Marking the build as aborted.
Build was aborted

Co-authored-by: gcf-merge-on-green[bot] <60162190+gcf-merge-on-green[bot]@users.noreply.github.com>

swegner reviewed May 6, 2016
View reviewed changes

jbonofre force-pushed the BEAM-254-RAT branch from c953a56 to dbaaf68Compare May 11, 2016 00:35

dhalperi reviewed May 20, 2016
View reviewed changes

jbonofre force-pushed the BEAM-254-RAT branch 2 times, most recently from 5d67cd3 to cadc8f9Compare May 23, 2016 18:46

davorbonaci reviewed May 23, 2016
View reviewed changes

jbonofre force-pushed the BEAM-254-RAT branch from cadc8f9 to 347033cCompare May 23, 2016 20:02

jbonofre force-pushed the BEAM-254-RAT branch from 347033c to e225754Compare May 23, 2016 20:06

jbonofre force-pushed the BEAM-254-RAT branch from e225754 to e3f06e1Compare May 24, 2016 12:45

jbonofre force-pushed the BEAM-254-RAT branch from e3f06e1 to 77d24ccCompare May 24, 2016 13:13

[BEAM-254] Add apache-rat-plugin execution and fix files with missing…
e26cf30
… license header

jbonofre force-pushed the BEAM-254-RAT branch from 77d24cc to e26cf30Compare May 31, 2016 17:19

asfgit merged commit e26cf30 into apache:masterMay 31, 2016

asfgit pushed a commit that referenced this pull request May 31, 2016
Thiscloses#297
6c338da

jbonofre deleted the BEAM-254-RAT branch February 24, 2017 11:55

iemejia pushed a commit to iemejia/beam that referenced this pull request Jan 12, 2018
Thisclosesapache#297
d7e0b34

pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
build: migrate to flakybot (apache#297)
ca7cc5b
Co-authored-by: gcf-merge-on-green[bot] <60162190+gcf-merge-on-green[bot]@users.noreply.github.com>

Uh oh!

Conversation

jbonofre commented May 6, 2016

Uh oh!

jbonofre commented May 6, 2016

Uh oh!

swegnerMay 6, 2016

Choose a reason for hiding this comment

Uh oh!

jbonofreMay 6, 2016

Choose a reason for hiding this comment

Uh oh!

swegner commented May 6, 2016

Uh oh!

jbonofre commented May 6, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kennknowles commented May 6, 2016

Uh oh!

jbonofre commented May 6, 2016

Uh oh!

jbonofre commented May 11, 2016

Uh oh!

jbonofre commented May 20, 2016

Uh oh!

dhalperiMay 20, 2016

Choose a reason for hiding this comment

Uh oh!

jbonofre commented May 21, 2016

Uh oh!

jbonofre commented May 23, 2016

Uh oh!

davorbonaci commented May 23, 2016

Uh oh!

davorbonaci commented May 23, 2016

Uh oh!

jbonofre commented May 23, 2016

Uh oh!

davorbonaciMay 23, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davorbonaciMay 23, 2016

Choose a reason for hiding this comment

Uh oh!

jbonofre commented May 23, 2016

Uh oh!

jbonofre commented May 23, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

davorbonaci commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

dhalperi commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

dhalperi commented May 24, 2016

Uh oh!

kennknowles commented May 24, 2016

Uh oh!

dhalperi commented May 24, 2016

Uh oh!

jbonofre commented May 24, 2016

Uh oh!

kennknowles commented May 24, 2016

Uh oh!

davorbonaci commented May 31, 2016

jbonofre commented May 6, 2016•
edited
Loading

davorbonaciMay 23, 2016•
edited
Loading