Skip to content

ARROW-5963: [R] R Appveyor job does not test changes in the C++ library#4900

Closed
nealrichardson wants to merge 8 commits intoapache:masterfrom
nealrichardson:r-appveyor-local
Closed

ARROW-5963: [R] R Appveyor job does not test changes in the C++ library#4900
nealrichardson wants to merge 8 commits intoapache:masterfrom
nealrichardson:r-appveyor-local

Conversation

@nealrichardson
Copy link
Member

@nealrichardson nealrichardson commented Jul 17, 2019

This changes the PKGBUILD script to use the local checkout to build the C++ library.

@nealrichardson
Copy link
Member Author

nealrichardson commented Jul 17, 2019

Here's a passing appveyor build: https://ci.appveyor.com/project/nealrichardson/arrow/builds/26053084

wesm
wesm approved these changes Jul 17, 2019
View reviewed changes
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. Probably want to rename this PKGBUILD file to something more descriptive, but can do that another time

@wesm wesm closed this in 1abf18f Jul 17, 2019
@nealrichardson
Copy link
Member Author

nealrichardson commented Jul 17, 2019

It's not a name of my choosing, it's what they're called everywhere: https://wiki.archlinux.org/index.php/PKGBUILD

cf. https://github.com/msys2/MINGW-packages

@nealrichardson nealrichardson deleted the r-appveyor-local branch July 17, 2019 22:28
@wesm
Copy link
Member

wesm commented Jul 17, 2019

Right, but you copy it here

https://github.com/apache/arrow/blob/master/ci/appveyor-build-r.sh#L42

so you could call it PKGBUILD_MINGW or something to people understand what it is by the name

@codecov-io
Copy link

codecov-io commented Jul 18, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4900      +/-   ##
==========================================
- Coverage   87.44%   87.43%   -0.01%     
==========================================
  Files         995      995              
  Lines      140465   140465              
  Branches     1418     1418              
==========================================
- Hits       122824   122817       -7     
- Misses      17279    17286       +7     
  Partials      362      362
Impacted Files Coverage Δ
cpp/src/arrow/csv/column-builder.cc 95.29% <0%> (-1.77%) ⬇️
cpp/src/arrow/io/readahead.cc 95.91% <0%> (-1.03%) ⬇️
cpp/src/plasma/thirdparty/ae/ae.c 70.75% <0%> (-0.95%) ⬇️
cpp/src/arrow/util/thread-pool-test.cc 97.66% <0%> (-0.94%) ⬇️
js/src/visitor/indexof.ts 98.63% <0%> (+1.36%) ⬆️

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 fc9d938...a87a8cf. Read the comment docs.

1 similar comment
@codecov-io
Copy link

codecov-io commented Jul 18, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4900      +/-   ##
==========================================
- Coverage   87.44%   87.43%   -0.01%     
==========================================
  Files         995      995              
  Lines      140465   140465              
  Branches     1418     1418              
==========================================
- Hits       122824   122817       -7     
- Misses      17279    17286       +7     
  Partials      362      362
Impacted Files Coverage Δ
cpp/src/arrow/csv/column-builder.cc 95.29% <0%> (-1.77%) ⬇️
cpp/src/arrow/io/readahead.cc 95.91% <0%> (-1.03%) ⬇️
cpp/src/plasma/thirdparty/ae/ae.c 70.75% <0%> (-0.95%) ⬇️
cpp/src/arrow/util/thread-pool-test.cc 97.66% <0%> (-0.94%) ⬇️
js/src/visitor/indexof.ts 98.63% <0%> (+1.36%) ⬆️

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 fc9d938...a87a8cf. Read the comment docs.

kszucs pushed a commit that referenced this pull request Jul 22, 2019
@nealrichardson @kszucs
ARROW-5963: [R] R Appveyor job does not test changes in the C++ library
5b4bcfe 
This changes the PKGBUILD script to use the local checkout to build the C++ library.

Author: Neal Richardson <[email protected]>

Closes #4900 from nealrichardson/r-appveyor-local and squashes the following commits:

a87a8cf <Neal Richardson> Revert "Test only mine"
16b7e56 <Neal Richardson> More playing with working directory
ed14a37 <Neal Richardson> Expand path
f0bf364 <Neal Richardson> Try this
eef10c6 <Neal Richardson> Oops
ec8aebf <Neal Richardson> Try without checksum
4fa5f70 <Neal Richardson> Test only mine
214a4fe <Neal Richardson> Try building C++ lib from local git checkout
@asfimport asfimport mentioned this pull request Aug 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wesm wesm wesm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nealrichardson @wesm @codecov-io