Skip to content

PARQUET-540: Fix Cascading 3 build thrift and SLF4J. - #328

Closed
rdblue wants to merge 2 commits into
apache:masterfrom
rdblue:PARQUET-540-fix-cascading3-build
Closed

rdblue wants to merge 2 commits into
apache:masterfrom
rdblue:PARQUET-540-fix-cascading3-build

Conversation

@rdblue

@rdblue rdblue commented Feb 19, 2016

Copy link
Copy Markdown
Contributor

This fixes:

  • parquet-cascading3 should have a libthrift dependency that uses thrift.version
  • parquet-cascading3 should have the standard SLF4J dependencies
  • Twitter's maven repo is no longer necessary because Parquet uses the Apache maven-thrift-plugin

@rdblue
rdblue force-pushed the PARQUET-540-fix-cascading3-build branch from a382668 to de2eb01 Compare April 17, 2016 00:18
This also updates the build to use the public maven-thrift-plugin, which
is version 0.1.11.
@rdblue
rdblue force-pushed the PARQUET-540-fix-cascading3-build branch from de2eb01 to c976e04 Compare April 17, 2016 00:54
@rdblue

rdblue commented Apr 18, 2016

Copy link
Copy Markdown
Contributor Author

@danielcweeks, can you take a look at this? Thanks!

@rdblue

rdblue commented May 11, 2016

Copy link
Copy Markdown
Contributor Author

@julienledem, can you look at this?

Cascading 3 broke the build for thrift 9. This fixes the problem. Thanks!

Comment thread parquet-cascading/pom.xml Outdated
<groupId>org.apache.thrift.tools</groupId>
<artifactId>maven-thrift-plugin</artifactId>
<version>0.1.10</version>
<version>0.1.11</version>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

pull this version constant out to the root pom file?

@piyushnarang

Copy link
Copy Markdown

minor comment, apart from that lgtm

@rdblue

rdblue commented Jun 30, 2016

Copy link
Copy Markdown
Contributor Author

@julienledem, since you're reviewing today, could you take a quick look at this one? It's a big annoyance. :)

@danielcweeks

Copy link
Copy Markdown

+1

@asfgit asfgit closed this in a421d95 Jul 15, 2016
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