Skip to content

ARROW-7169: [C++] Vendor uriparser library - #5865

Closed
pitrou wants to merge 1 commit into
apache:masterfrom
pitrou:ARROW-7169-vendor-uriparser
Closed

ARROW-7169: [C++] Vendor uriparser library#5865
pitrou wants to merge 1 commit into
apache:masterfrom
pitrou:ARROW-7169-vendor-uriparser

Conversation

@pitrou

Copy link
Copy Markdown
Member

The library is only used internally (the types and headers are not exposed).
Vendoring it makes it easier for core functionality to depend on it.

@pitrou

Copy link
Copy Markdown
MemberAuthor

@kou Do you have any concerns?

@pitrou
pitrouforce-pushed the ARROW-7169-vendor-uriparser branch from 095988c to 7abf071CompareNovember 19, 2019 15:54
@github-actions

Copy link
Copy Markdown

@pitrou
pitrouforce-pushed the ARROW-7169-vendor-uriparser branch 3 times, most recently from 01cf4b2 to 0a1a1daCompareNovember 19, 2019 19:37
kou
kou approved these changes Nov 20, 2019

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Comment threadcpp/src/arrow/vendored/uriparser/UriConfig.h.in Outdated
The library is only used internally (the types and headers are not exposed).
Vendoring it makes it easier for core functionality to depend on it.
@pitrou
pitrouforce-pushed the ARROW-7169-vendor-uriparser branch from 0a1a1da to 83bb7c2CompareNovember 20, 2019 12:12

@kszucskszucs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@kszucs

Copy link
Copy Markdown
Member

@pitrou thanks for updating the packaging builds as well

@pitrou

Copy link
Copy Markdown
MemberAuthor

@kszucs Yes, I had forgotten to update some of them for the double-conversion vendoring.

@kszucs

Copy link
Copy Markdown
Member

Our nightlies are in pretty bad shape, so it would not make much sense to run them. Merging.

@pitrou
pitrou deleted the ARROW-7169-vendor-uriparser branch November 20, 2019 13:34
Sign up for freeto 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

@pitrou@kszucs@kou