renkun-ken commented 2 years ago

Closes #51

This PR adds timeout support to connection config.

richfitz commented 2 years ago

Thanks for this! I've made a couple of small changes here:

8cc49b9: this test must be skipped on CRAN, and was/is also failing on non-linux systems. This version might work, we'll see.
5a2ef85: this did some fairly boring style changes and deduplicated some of the R/C wrangling

Apologies for directly pushing to your branch, but trying to coordinate getting this to CRAN ~Monday

renkun-ken commented 2 years ago

Thanks for the review and pushing those changes. Feel free to modify if needed.

codecov-commenter commented 2 years ago

Codecov Report

Merging #52 (f0ce5b6) into master (5b24b02) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head f0ce5b6 differs from pull request most recent head 796698f. Consider uploading reports for the commit 796698f to get more accurate results

@@            Coverage Diff            @@
##            master       #52   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1479      1490   +11     
=========================================
+ Hits          1479      1490   +11

Impacted Files	Coverage Δ
src/registration.c	`100.00% <ø> (ø)`
R/config.R	`100.00% <100.00%> (ø)`
R/redis.R	`100.00% <100.00%> (ø)`
src/connection.c	`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 5b24b02...796698f. Read the comment docs.

richfitz / redux

Connect timeout #52

Codecov Report