Skip to content

Conversation

@kou
Copy link
Member

@kou kou commented Sep 11, 2018

It's already disabled in MSVC build.

It's already disabled in MSVC build.
@kou
Copy link
Member Author

kou commented Sep 11, 2018

You can see the important changes easily by disabling whitespace changes by https://github.com/apache/arrow/pull/2539/files?w=1 .

@kou kou changed the title ARROW-3124: [C++] Disable insecure warnings in MinGW build ARROW-3214: [C++] Disable insecure warnings in MinGW build Sep 11, 2018
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM, just one nit below.

set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /bigobj")
# Support large object code
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /bigobj")
endif()
Copy link
Member

Choose a reason for hiding this comment

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

Writing this as endif(MSVC) would make things easier to follow, IMHO.

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense.
I've added.

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

@xhochy xhochy closed this in 8393119 Sep 12, 2018
@kou kou deleted the cpp-disable-insecure-warnings-with-mingw branch September 13, 2018 01:24
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