Skip to content

Conversation

@sbinet
Copy link
Contributor

@sbinet sbinet commented Jun 29, 2018

No description provided.

@codecov-io
Copy link

codecov-io commented Jun 29, 2018

Codecov Report

Merging #2193 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2193      +/-   ##
==========================================
- Coverage   86.47%   86.47%   -0.01%     
==========================================
  Files         244      244              
  Lines       41808    41835      +27     
==========================================
+ Hits        36155    36178      +23     
- Misses       5653     5657       +4
Impacted Files Coverage Δ
cpp/src/arrow/python/helpers.cc 79.87% <0%> (-3.23%) ⬇️
cpp/src/arrow/python/common.h 100% <0%> (ø) ⬆️
cpp/src/arrow/python/python_to_arrow.cc 88.29% <0%> (+0.12%) ⬆️
cpp/src/arrow/python/arrow_to_python.cc 91.71% <0%> (+0.2%) ⬆️
cpp/src/arrow/python/builtin_convert.cc 92.1% <0%> (+0.47%) ⬆️
cpp/src/arrow/util/thread-pool-test.cc 99.45% <0%> (+0.54%) ⬆️
cpp/src/arrow/python/helpers.h 90% <0%> (+1.11%) ⬆️

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 c9ce215...a2dd243. Read the comment docs.

@wesm
Copy link
Member

wesm commented Jun 29, 2018

Thanks @sbinet! We should also integrate this into https://github.com/apache/arrow/blob/master/ci/detect-changes.py so the Go tests are only run when there is a Go change.

@sbinet
Copy link
Contributor Author

sbinet commented Jun 29, 2018

should I integrate this into this PR or should I create a JIRA ticket+new PR ?

@wesm
Copy link
Member

wesm commented Jun 29, 2018

You can do it here, no problem

@sbinet
Copy link
Contributor Author

sbinet commented Jun 29, 2018

Ok. PTAL.

@sbinet
Copy link
Contributor Author

sbinet commented Jun 29, 2018

not sure why appveyor failed (in C++, I think) nor why suddenly the travis-ruby test failed (loading some lua library)...

@sbinet
Copy link
Contributor Author

sbinet commented Jun 29, 2018

(but this seems unrelated to my change.)

.travis.yml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a line - if [ $ARROW_CI_GO_AFFECTED != "1" ]; then exit; fi here so that the Go build only runs for GO PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will do when in front of a real keyboard.

Copy link
Member

Choose a reason for hiding this comment

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

Rebased and added that

@sbinet
Copy link
Contributor Author

sbinet commented Jul 2, 2018

@wesm thanks!

the travis issue seems unrelated.

@pitrou
Copy link
Member

pitrou commented Jul 2, 2018

@sbinet yes, the test_use_huge_pages failure in unrelated.

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.

+1. Thanks for doing this @sbinet !

@pitrou
Copy link
Member

pitrou commented Jul 2, 2018

Just for my personal enlightenment: when go install runs in 0.10s, is it really the entire build time for Arrow? :-o

@pitrou pitrou closed this in ceae13d Jul 2, 2018
@sbinet
Copy link
Contributor Author

sbinet commented Jul 2, 2018

Yep. Go builds really fast. :)
(Also, Go arrow is rather bare bones for now...)

@wesm
Copy link
Member

wesm commented Jul 2, 2018

The way I look at Go, it's Google's reaction to a decade of production C++ development. A lot of Google C++ best practices are reflected in the language itself, and the difference in compiler speed of course is a stark difference to large C++ projects

@sbinet sbinet deleted the issue-2344 branch July 2, 2018 21:41
pitrou pushed a commit that referenced this pull request Jul 3, 2018
needs #2193

Author: Sebastien Binet <binet@cern.ch>

Closes #2203 from sbinet/issue-2780 and squashes the following commits:

59c71bb <Sebastien Binet> ARROW-2780:  Run code coverage analysis
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.

5 participants