'.import.rio_xls()' no longer passing '...' to 'read_xls()' #230

bokov · 2019-09-23T22:15:36Z

because that function cannot accept '...' arguments. Instead, the length of '...' is checked and if > 0 a warning is issued. With corresponding tests. Also went back and changed my earlier 'read_xlsx()' test to be 'expect_warning()' instead of 'expect_true()' since by that point that's what needs testing.

Please ensure the following before submitting a PR:

if suggesting code changes or improvements, open an issue first

This addresses the final part of #223 , #224 , and item 2 in #226

for all but trivial changes (e.g., typo fixes), add your name to DESCRIPTION
for all but trivial changes (e.g., typo fixes), documentation your change in NEWS.md with a parenthetical reference to the issue number being addressed

Awaiting guidance

if changing documentation, edit files in /R not /man and run devtools::document() to update documentation
No changes
add code or new test files to /tests for any new functionality or bug fix
make sure R CMD check runs without error before submitting the PR
No new errors.


        '.import.rio_xls()' no longer passing '...' to 'read_xls()' because t…

…hat functions cannot accept '...' arguments. Instead, the length of '...' is checked and if > 0 a warning is issued. With corresponding tests. Also went back and changed my earlier 'read_xlsx()' test to be 'expect_warning()' instead of 'expect_true()' since by that point that's what needs testing.

codecov-io · 2019-09-23T22:25:54Z

Codecov Report

Merging #230 into master will decrease coverage by 0.1%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
- Coverage   82.43%   82.32%   -0.11%     
==========================================
  Files          18       18              
  Lines         871      877       +6     
==========================================
+ Hits          718      722       +4     
- Misses        153      155       +2

Impacted Files	Coverage Δ
R/import_methods.R	`79.59% <86.66%> (-0.41%)`	⬇️

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 70d26ca...c3b85d4. Read the comment docs.

leeper

Looks good - a couple of small changes requested. Thanks for this!

R/import_methods.R


        Removed unneeded call to 'requireNamespace()' and fixed formatting as…

… per (/230#pullrequestreview-296156610)

bokov · 2019-10-02T16:32:32Z

I re-ran this commit on my own Travis account and the problem seems to have resolved itself
https://travis-ci.org/bokov/rio/builds/592605884

Appveyor is still failing to install packages 'haven', 'curl', 'data.table', 'readxl', 'openxlsx', 'tibble'
for Environment: R_VERSION=devel, R_ARCH=x64, GCC_PATH=mingw_64... including for the head of the master branch of rio.

bokov referenced this pull request Sep 30, 2019

Using the same argument filters as #222 for .import.rio_rdata, .import.rio_xls, and .import.rio_ods #223

Open

1 of 3 tasks complete

leeper requested changes Oct 2, 2019

View changes

R/import_methods.R Outdated Show resolved Hide resolved

R/import_methods.R Outdated Show resolved Hide resolved

Removed unneeded call to 'requireNamespace()' and fixed formatting as…

Loading status checks…

c3b85d4

… per (/230#pullrequestreview-296156610)

bokov requested a review from leeper Oct 2, 2019

leeper merged commit b9f78fa into leeper:master Oct 3, 2019
0 of 2 checks passed

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

continuous-integration/appveyor/pr AppVeyor build failed
Details

leeper/rio

'.import.rio_xls()' no longer passing '...' to 'read_xls()' #230

'.import.rio_xls()' no longer passing '...' to 'read_xls()' #230

bokov commented Sep 23, 2019

This comment has been minimized.

codecov-io commented Sep 23, 2019 •

edited

leeper left a comment

This comment has been minimized.

bokov commented Oct 2, 2019

leeper/rio

Join GitHub today

'.import.rio_xls()' no longer passing '...' to 'read_xls()' #230

Conversation

bokov commented Sep 23, 2019

This comment has been minimized.

codecov-io commented Sep 23, 2019 • edited

Codecov Report

leeper left a comment

This comment has been minimized.

bokov commented Oct 2, 2019

codecov-io commented Sep 23, 2019 •

edited