Skip to content

ARROW-4423: [C++] Upgrade vendored gmock/gtest to 1.8.1#3589

Closed
emkornfield wants to merge 4 commits intoapache:masterfrom
emkornfield:upgrade_gmock
Closed

ARROW-4423: [C++] Upgrade vendored gmock/gtest to 1.8.1#3589
emkornfield wants to merge 4 commits intoapache:masterfrom
emkornfield:upgrade_gmock

Conversation

@emkornfield
Copy link
Contributor

@emkornfield emkornfield commented Feb 8, 2019

This change seems to indicate an alternative would be to force the build to be Release type (and avoid the naming magic). It doesn't appear that the conda feedstock (https://github.com/conda-forge/gtest-feedstock/blob/master/recipe/build.sh) builds debug for linux at all. it does for windows though:
(https://github.com/conda-forge/gtest-feedstock/blob/master/recipe/bld.bat) I'm not sure if we care a lot about this?

@xhochy
Copy link
Member

xhochy commented Feb 8, 2019

Rebased.

@wesm
Copy link
Member

wesm commented Feb 9, 2019

I can try to look at the Windows build later if you can't figure it out. BTW you can get free Windows 10 ISO from Microsoft so setting up a windows dev environment isn't as hard as you might think

@emkornfield
Copy link
Contributor Author

emkornfield commented Feb 10, 2019 via email

@emkornfield emkornfield changed the title ARROW-4423: [WIP][C++] Upgrade vendored gmock to 1.8.1 ARROW-4423: [C++] Upgrade vendored gmock to 1.8.1 Feb 11, 2019
@emkornfield emkornfield changed the title ARROW-4423: [C++] Upgrade vendored gmock to 1.8.1 ARROW-4423: [C++] Upgrade vendored gmock/gtest to 1.8.1 Feb 11, 2019
@codecov-io
Copy link

Codecov Report

Merging #3589 into master will increase coverage by 0.85%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3589      +/-   ##
==========================================
+ Coverage   87.76%   88.62%   +0.85%     
==========================================
  Files         673      544     -129     
  Lines       82763    73796    -8967     
  Branches     1069        0    -1069     
==========================================
- Hits        72640    65400    -7240     
+ Misses      10012     8396    -1616     
+ Partials      111        0     -111
Impacted Files Coverage Δ
cpp/src/arrow/csv/column-builder.cc 95.39% <0%> (-1.98%) ⬇️
cpp/src/plasma/thirdparty/ae/ae.c 71.09% <0%> (-0.95%) ⬇️
go/arrow/math/uint64_amd64.go
go/arrow/memory/memory_avx2_amd64.go
js/src/enum.ts
go/arrow/array/builder.go
js/src/Arrow.node.ts
js/src/schema.ts
go/arrow/type_traits_boolean.go
js/src/ipc/node/writer.ts
... and 121 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f13b70...200dec7. Read the comment docs.

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

Leaving merging to @wesm as he reported problems with gtest recently (that have hopefully been resolved).

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. I'll rebase my ARROW-47 patch on this

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.

4 participants