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

Added tar_renv #170

Merged
merged 5 commits into from
Sep 23, 2020
Merged

Added tar_renv #170

merged 5 commits into from
Sep 23, 2020

Conversation

mattwarkentin
Copy link
Contributor

@mattwarkentin mattwarkentin commented Sep 22, 2020

Prework

Summary

Add the tar_renv() function and documentation as discussed in #157. PR also includes unit tests and the addition of tar_renv to _pkdgown.yml.

Related GitHub issues and pull requests

Checklist

  • This pull request is not a draft.

@mattwarkentin
Copy link
Contributor Author

── R CMD check results ───────────────────────────────── targets 0.0.0.9000 ────
Duration: 1m 39s

❯ checking for future file timestamps ... NOTE
  unable to verify current time

0 errors ✔ | 0 warnings ✔ | 1 note ✖

@mattwarkentin
Copy link
Contributor Author

Still one more lint complaint at https://github.com/wlandau/targets/pull/170/files#diff-2952800a4992b3e5427b2bad378bfd3fR31

I think this one is unavoidable...

Copy link
Member

@wlandau wlandau left a comment

Choose a reason for hiding this comment

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

This is off to a great start, thanks for contributing. I know I have a lot of comments, but they are all minor.

By the way, feel free to add yourself as a contributor in the DESCRIPTION file, e.g. person(given = "Matthew", family = "Warkentin", role = "ctb").

R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
R/tar_renv.R Outdated Show resolved Hide resolved
@wlandau
Copy link
Member

wlandau commented Sep 22, 2020

Also, don't worry about the check note in #170 (comment). That one is truly unavoidable.

@mattwarkentin
Copy link
Contributor Author

Thanks for all the comments! No sirens went off this time, so I must be learning how to do all my checks BEFORE committing the code.

@wlandau
Copy link
Member

wlandau commented Sep 23, 2020

Looks great, thanks for your work.

@wlandau wlandau merged commit e3b5464 into ropensci:master Sep 23, 2020
@mattwarkentin
Copy link
Contributor Author

Thanks for allowing me to go through this process. You probably spent more time reviewing my code than it would've taken you to implement this feature yourself, so I do appreciate it.

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.

2 participants