Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(issue #64) add always_list parameter (fparse/fload) #65

Merged
merged 3 commits into from
Feb 10, 2021
Merged

Conversation

knapply
Copy link
Collaborator

@knapply knapply commented Feb 10, 2021

Resolves #64.

@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #65 (e7d5cd4) into master (32320b9) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   96.18%   96.21%   +0.02%     
==========================================
  Files          18       18              
  Lines        1363     1373      +10     
==========================================
+ Hits         1311     1321      +10     
  Misses         52       52              
Impacted Files Coverage Δ
R/fload.R 100.00% <100.00%> (ø)
R/fparse.R 100.00% <100.00%> (ø)

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 32320b9...e7d5cd4. Read the comment docs.

Copy link
Owner

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Looks good as usual! I can deal with the codecov nag separately.

I think I would have put the new named argument at the end by some time honoured convention though ...

@knapply
Copy link
Collaborator Author

knapply commented Feb 10, 2021

Looks good as usual! I can deal with the codecov nag separately.

I think I would have put the new named argument at the end by some time honoured convention though ...

The codecov should be fixed momentarily. The rationale behind the argument order is that the arguments that are applicable to both fparse()/fload() go before the ones that are only applicable to fload(), but I can change that if desired.

@eddelbuettel
Copy link
Owner

Makes sense, I didn't pick up on that.

R/fload.R Show resolved Hide resolved
@tdeenes
Copy link

tdeenes commented Feb 10, 2021

Thank you very much for the open-mindedness and the quick implementation!

@eddelbuettel eddelbuettel merged commit e4207c5 into master Feb 10, 2021
@eddelbuettel eddelbuettel deleted the issues/64 branch February 20, 2021 14:05
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.

Add 'enforce_lapply' or similar to fparse/fload
3 participants