Skip to content

[BEAM-3457] Drop Go Maven PreCommit#4819

Merged
lukecwik merged 1 commit intoapache:masterfrom
herohde:maven
Mar 9, 2018
Merged

[BEAM-3457] Drop Go Maven PreCommit#4819
lukecwik merged 1 commit intoapache:masterfrom
herohde:maven

Conversation

@herohde
Copy link
Contributor

@herohde herohde commented Mar 7, 2018

From #4814: For Go, the Gradle build passed 100% of the time while the Maven build passed 95% of the time in the past week. Both take ~4 mins to build.

Copy link
Member

@kennknowles kennknowles left a comment

Choose a reason for hiding this comment

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

These changes are not tested so there is no signal gained by waiting.

@kennknowles
Copy link
Member

I don't want to mess with the seed job right now just because of the large reverts going on (shouldn't affect them but just a little hesitant to touch anything that could delay them)

@kennknowles
Copy link
Member

Actually on #4818 the RAT check in Maven caught something that the Gradle build did not. So we should have confirmation that RAT is catching everything in gradle. TBH I think that could be a separate Jenkins job since it is language agnostic and with no upstream or downstream steps.

This is a demonstration that the goal of precommit is not to succeed but to fail at the right times :-)

@herohde
Copy link
Contributor Author

herohde commented Mar 9, 2018

@kennknowles -- is there a blocker to merging? The Go PreCommit shouldn't do the RAT check, I assume.

@lukecwik
Copy link
Member

lukecwik commented Mar 9, 2018

The RAT run is fixed in #4830

@lukecwik lukecwik merged commit b408d48 into apache:master Mar 9, 2018
@kennknowles
Copy link
Member

There was no blocker from me - just wanted to confirm that we didn't have an important coverage gap, which Luke checked.

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.

3 participants