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.

Sign up for GitHub

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

Jump to bottom

Removed closeAllConnections from tests (fixes #135) #136

Merged
merged 6 commits into from
Feb 1, 2019
Merged

Removed closeAllConnections from tests (fixes #135) #136

merged 6 commits into from
Feb 1, 2019

Conversation

jameslamb
Copy link
Collaborator

This should get CRAN off our backs. If you approve I'll cut a release and push it, but let's keep the PR open until we get accepted.

austin3dickey
austin3dickey approved these changes Jan 28, 2019
View reviewed changes
Copy link
Collaborator

@austin3dickey austin3dickey left a comment

Choose a reason for hiding this comment

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

lgtm

@jameslamb
Copy link
Collaborator Author

I had to add testthat, rmarkdown, and knitr to the build. Think they must have made the default Travis env for R more lightweight.

@jameslamb
Copy link
Collaborator Author

submitted 0.3.1 to CRAN. We should hear back tomorrow

@jameslamb
Copy link
Collaborator Author

oh I know why CRAN failed on my submission! I forgot we moved the test data out to a shared folder. I'll update CONTRIBUTING.md and add a build thing in the Makefile

@jameslamb jameslamb mentioned this pull request Jan 30, 2019
Merged
austin3dickey
austin3dickey reviewed Jan 30, 2019
View reviewed changes
CONTRIBUTING.md Outdated Show resolved Hide resolved
@jameslamb
Copy link
Collaborator Author

The build failure should be fixed when we merge #138 and rebase this (I'll do that soon). Couple things I had to fix that are unrelated to this and just related to the fact that Travis has changed under our feet in the last couple months

@jameslamb
Copy link
Collaborator Author

@austin3dickey I updated cran-comments.md and also updated the pkgdown site (seemed like the right thing to do for a release commit)

@codecov-io
Copy link

codecov-io commented Feb 1, 2019 •
edited
Loading

Codecov Report

Merging #136 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #136   +/-   ##
=======================================
  Coverage   95.23%   95.23%           
=======================================
  Files           8        8           
  Lines         608      608           
=======================================
  Hits          579      579           
  Misses         29       29

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 ced8bb4...0973bbb. Read the comment docs.

@austin3dickey
Copy link
Collaborator

looks great!

@jameslamb jameslamb merged commit ff0b9ea into uptake:master Feb 1, 2019
@jameslamb jameslamb deleted the bugfix/cran_stuff branch June 1, 2021 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austin3dickey austin3dickey austin3dickey approved these changes

Assignees

@austin3dickey austin3dickey

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jameslamb @codecov-io @austin3dickey