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

PARQUET-1431: [C++] Automaticaly set thrift to use boost for thrift versions before 0.11 #2645

Closed
wants to merge 3 commits into from

Conversation

majetideepak
Copy link
Contributor

@majetideepak majetideepak commented Sep 27, 2018

No description provided.

@majetideepak majetideepak force-pushed the PARQUET-1431 branch 2 times, most recently from 5453401 to ca8fbae Compare September 28, 2018 11:32
@wesm wesm changed the title PARQUET-1431: [C++] Automaticaly set thrift to use boost for thrift v… PARQUET-1431: [C++] Automaticaly set thrift to use boost for thrift versons before 0.11 Sep 29, 2018
Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1, rebased. Will merge after build runs

Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

+1, LGTM

@wesm
Copy link
Member

wesm commented Sep 29, 2018

This is broken, taking a look

@wesm
Copy link
Member

wesm commented Sep 29, 2018

I have no idea why THRIFT_VERSION VERSION_LESS "0.11.0" is not working properly. I've spent the last 10 minutes twiddling with it and do not know what is wrong

Change-Id: Iec4e16517021f26cb957d9fc013be6e01b411cfc
@wesm
Copy link
Member

wesm commented Sep 29, 2018

Oh I see we need to do a regex

Change-Id: Icf773cf0f1456ca7ecec13ce9f105f9413df3fc1
@majetideepak majetideepak changed the title PARQUET-1431: [C++] Automaticaly set thrift to use boost for thrift versons before 0.11 PARQUET-1431: [C++] Automaticaly set thrift to use boost for thrift versions before 0.11 Sep 29, 2018
@majetideepak
Copy link
Contributor Author

+1 LGTM. Thanks or noticing the regex requirement @wesm.

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