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

Back-compatible custom key manglers #90

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

wlandau
Copy link
Contributor

@wlandau wlandau commented Nov 6, 2018

So far, this PR is just a sketch of #89 (comment). It came together quicker than I predicted. @richfitz, if you agree with implementation, then I will write tests.

@codecov-io
Copy link

codecov-io commented Nov 6, 2018

Codecov Report

Merging #90 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #90      +/-   ##
==========================================
+ Coverage   99.91%   99.91%   +<.01%     
==========================================
  Files          15       15              
  Lines        1179     1236      +57     
==========================================
+ Hits         1178     1235      +57     
  Misses          1        1
Impacted Files Coverage Δ
R/driver_remote.R 100% <100%> (ø) ⬆️
R/utils.R 100% <100%> (ø) ⬆️
R/driver_rds.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 2750823...d0bb797. Read the comment docs.

@wlandau
Copy link
Contributor Author

wlandau commented Nov 6, 2018

This morning, I added bug fixes, tests, and documentation. I think this PR is ready for an initial review.

@richfitz
Copy link
Owner

richfitz commented Nov 8, 2018

Just to let you know - this is absoutely on my radar, but this week has been busy. I will look at this asap

@wlandau
Copy link
Contributor Author

wlandau commented Nov 9, 2018

Thanks, Rich. FYI I will be on vacation from Nov 15 to 27, so I do not expect this work to be fast.

@richfitz richfitz self-requested a review November 9, 2018 09:12
@wlandau
Copy link
Contributor Author

wlandau commented Dec 14, 2018

@richfitz, what are your current thoughts on custom key manglers? Do you still this PR's approach to #88 is a good idea?

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.

4 participants