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

adds host to formals of install_theme() and new_site() #265

Merged
merged 6 commits into from Feb 26, 2018

Conversation

Projects
None yet
2 participants
@ijlyttle
Copy link
Contributor

ijlyttle commented Feb 25, 2018

This is just to open the conversation - not finished yet. When done, this fixes #264.

When I add the argument host, I get a warning on R CMD check:

* checking Rd \usage sections ... WARNING
Duplicated \argument entries in documentation object 'hugo_cmd':
  ‘host’

To get around this, I could use hostname instead.

@ijlyttle

This comment has been minimized.

Copy link
Contributor Author

ijlyttle commented Feb 25, 2018

The coding is complete (I hope) - it will take me a little time to make sure that everything actually works.

@yihui

yihui approved these changes Feb 25, 2018

Copy link
Member

yihui left a comment

The code looks good to me. Could you either leave out the changes in Rd (I'll roxygenize later), or install.packages('Rd2roxygen') and use Cmd + Shift + B (i.e. Install and Restart) to generate Rd? Sorry I use a obscure way to roxygenize the package.

R/hugo.R Outdated
@@ -92,7 +94,8 @@ change_config = function(name, value) {
#' if (interactive()) new_site()
new_site = function(
dir = '.', install_hugo = TRUE, format = 'toml', sample = TRUE,
theme = 'yihui/hugo-lithium-theme', theme_example = TRUE, serve = interactive()
theme = 'yihui/hugo-lithium-theme', theme_example = TRUE, serve = interactive(),
hostname = 'github.com'

This comment has been minimized.

Copy link
@yihui

yihui Feb 25, 2018

Member

I think it makes a little more sense to move this argument closer to theme (e.g. right after theme).

This comment has been minimized.

Copy link
@ijlyttle

ijlyttle Feb 25, 2018

Author Contributor

will do!

@ijlyttle

This comment has been minimized.

Copy link
Contributor Author

ijlyttle commented Feb 25, 2018

I will adjust the roxygen process - if I can not get to do what you want it to do, I will leave out the changes in Rd files.

@ijlyttle

This comment has been minimized.

Copy link
Contributor Author

ijlyttle commented Feb 25, 2018

@yihui - I have rebuilt the .Rd files - just to be sure that the result is as you expect - thanks!

@yihui

yihui approved these changes Feb 26, 2018

Copy link
Member

yihui left a comment

Looks perfect now! The next thing to do is to add yourself as ctb to DESCRIPTION (alphabetical order by first name), and an item in NEWS.md. Thanks!

@ijlyttle

This comment has been minimized.

Copy link
Contributor Author

ijlyttle commented Feb 26, 2018

I gave it a quick test, and it works as expected for me, both for GitHub and GHE. Will make those additions right away. Thanks!

adds @ijlyttle to contributors
adds bullet point to NEWS.md (corrects typo in previous bullet)
@ijlyttle

This comment has been minimized.

Copy link
Contributor Author

ijlyttle commented Feb 26, 2018

should be good-to-go!

@@ -2,7 +2,9 @@

## NEW FEATURES

- Added a `title_case` argument to the `new_post()` function; if \code{TRUE}, the post title will be converted to title case. See `?blogdown::new_post` for details.
- Added a `title_case` argument to the `new_post()` function; if `TRUE`, the post title will be converted to title case. See `?blogdown::new_post` for details.

This comment has been minimized.

Copy link
@yihui

yihui Feb 26, 2018

Member

Nice catch. Thanks!

@yihui yihui merged commit 95f694d into rstudio:master Feb 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yihui yihui added this to the v0.6 milestone Feb 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.