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

Export use_directory() and wrote documentation for it #44

Merged
merged 2 commits into from
Aug 11, 2017
Merged

Export use_directory() and wrote documentation for it #44

merged 2 commits into from
Aug 11, 2017

Conversation

s-fleck
Copy link
Contributor

@s-fleck s-fleck commented Aug 10, 2017

No description provided.

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

Can you please also:

  • Add a bullet to NEWS.md following the existing format

  • Ensure that on the commits includes Fixes #27 in the commit message so that the corresponding issue is closed when this PR is merge

R/helpers.R Outdated
#'
#' @return `TRUE` (invisibly) if the directory was successfully created.
#'
#' @inheritParams use_template
Copy link
Member

Choose a reason for hiding this comment

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

Also need to document path

R/helpers.R Outdated
#' package root dir. This function powers many of the other `use_` functions
#' such as [use_data()] and [use_vignette()].
#'
#' @return `TRUE` (invisibly) if the directory was successfully created.
Copy link
Member

Choose a reason for hiding this comment

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

I think you can leave this off. I'd rather do it for all function in one swoop

@hadley hadley mentioned this pull request Aug 11, 2017
* Small fixes to the use_directory() roxygen
* Added use_directory() export to NEWS.md
@s-fleck
Copy link
Contributor Author

s-fleck commented Aug 11, 2017

Hmm im kinda new to this. To me it shows two commits for the pull request now: the initial one, and the one that adds documentation for path and adds the NEWS.md bullet point.

@hadley
Copy link
Member

hadley commented Aug 11, 2017

Hmmm, now it looks mostly ok. I'll finish off the process. Thanks!

@s-fleck
Copy link
Contributor Author

s-fleck commented Aug 11, 2017

Ok thanks, no idea what went wrong on the initial push

@hadley hadley merged commit 57fd240 into r-lib:master Aug 11, 2017
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