Skip to content

ARROW-1336: [C++] Add arrow::schema factory function, simply some awkward constructors #948

Closed
wesm wants to merge 1 commit intoapache:masterfrom
wesm:ARROW-1336
Closed

ARROW-1336: [C++] Add arrow::schema factory function, simply some awkward constructors #948
wesm wants to merge 1 commit intoapache:masterfrom
wesm:ARROW-1336

Conversation

@wesm
Copy link
Member

@wesm wesm commented Aug 8, 2017

No description provided.

@wesm
Copy link
Member Author

wesm commented Aug 8, 2017

cc @cpcloud

I'm not sure why Appveyor is upset

…initializer list

Change-Id: Ia82a946f549c5b28ffad20ae4a0014d007e098de
@pcmoritz
Copy link
Contributor

pcmoritz commented Aug 8, 2017

Have you considered naming it make_schema? That'd go well with std::make_unique, std::make_shared, std::make_pair, etc; then you can also get rid of ::arrow:: (I assume that was to avoid name clashes).

Copy link
Contributor

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

LGTM

@wesm
Copy link
Member Author

wesm commented Aug 8, 2017

@pcmoritz I did think of that; this is more consistent with functions like arrow::field, arrow::timestamp, etc. I would rather change everything than do something inconsistent, so perhaps we can revisit if there are strong feelings about the naming style of these factory functions

I wonder what is going on with the segfault in Travis CI, we should try to figure that out (I have seen it before): opned ARROW-1338

@asfgit asfgit closed this in 02ab748 Aug 8, 2017
@wesm wesm deleted the ARROW-1336 branch August 8, 2017 02:06
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