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

start refactor of digest #116

Merged
merged 2 commits into from Sep 21, 2019

Conversation

@mdequeljoe
Copy link
Contributor

commented Sep 20, 2019

This PR picks up on part 1 of #115 by using the common helper functions/if-blocks in digest() that are also used in vdigest(). These functions are put in a 'utils.R' file.

@codecov

This comment has been minimized.

Copy link

commented Sep 20, 2019

Codecov Report

Merging #116 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #116   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          21     22    +1     
  Lines        1722   1703   -19     
=====================================
- Hits         1722   1703   -19
Impacted Files Coverage Δ
R/vdigest.R 100% <ø> (ø) ⬆️
R/digest.R 100% <100%> (ø) ⬆️
R/utils.R 100% <100%> (ø)

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 04e9a57...161d26e. Read the comment docs.

Copy link
Owner

left a comment

Nice work

I'm old school so see how I add to ChangeLog -- for us Emacs users: 'C-x 4 a; automates it, for the rest of you all it is a chore ;-) Maybe add one next time. But I think this is ready (and I'll happily add the entry next round). But if you have a minute ...

@eddelbuettel eddelbuettel force-pushed the mdequeljoe:utils branch from a3be39c to 161d26e Sep 21, 2019
@eddelbuettel

This comment has been minimized.

Copy link
Owner

commented Sep 21, 2019

[ Because of the then-pending-at-CRAN release of 0.6.21 I had not committed / finalised my changes to DESCRIPTION and ChangeLog, so historically they were "before" your changes here, effectively they were after -- as I just committed last night. So that made it a good candidate for git merge. Which I just did. That preserves your PR as two commits, but moves dates and changes sha1. The simpler alternative would have been a squash-merge but this we actually had a conflict -- my bad as I asked you to update ChangeLog, and also updated ChangeLog myself. But it looks like it all worked out. ]

@eddelbuettel eddelbuettel merged commit e9d0387 into eddelbuettel:master Sep 21, 2019
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 04e9a57
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.