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

Redesign the way we build a golem "here" #312

Open
ColinFay opened this issue Nov 28, 2019 · 0 comments
Open

Redesign the way we build a golem "here" #312

ColinFay opened this issue Nov 28, 2019 · 0 comments

Comments

@ColinFay
Copy link
Member

So, I think we need to change a little bit the way {golem} behaves when the user chooses to build a golem in his current directory.

Following the last changes in {shiny}, notably these two PR from @trestletech:

There's a good chance that users will end with an app structure that looks like:

app.R
L R /
  L this.R
  L that.R 
L tests/
  ...
L www (maybe)

This is very close to what we've got in {golem}, and I think that this should be facilitated by golem::create_golem(".").

The current implementation of this function throws a warning:

> golem::create_golem(".")
── Checking package name ───────────────────────────────────────────────────────
✔ Valid package name
The path . already exists, override?
1: Yeah
2: No way
3: Not yet

When it fact even if we say YES, the path is not really overriden: the files are copied, but the one that already existed are not deleted.

colin:plop colin$ cd /tmp && mkdir plouf && cd plouf
colin:plouf colin$ mkdir R tests www
colin:plouf colin$ touch app.R  R/this.R R/that.R www/ping.css
colin:plouf colin$ ls
R	app.R	tests	www
colin:plouf colin$ R --quiet
> golem::create_golem(".")
── Checking package name ───────────────────────────────────────────────────────
✔ Valid package name
The path . already exists, override?
1: No way
2: Yes
3: Nope

Selection: 2
── Creating dir ────────────────────────────────────────────────────────────────
✔ Created package directory
── Copying package skeleton ────────────────────────────────────────────────────
✔ Copied app skeleton
── Setting the default config ──────────────────────────────────────────────────
✔ Configured app
── Done ────────────────────────────────────────────────────────────────────────
A new golem named plouf was created at /private/tmp/plouf .
To continue working on your app, start editing the 01_start.R file.
> q()
Save workspace image? [y/n/c]: n
colin:plouf colin$ ls 
DESCRIPTION	R		dev		man		www
NAMESPACE	app.R		inst		tests
colin:plouf colin$ ls R 
app_config.R	app_server.R	app_ui.R	run_app.R	that.R		this.R

So, my take on that kind of app is:

  • We should not ask to "override" as it is not really what is done
  • We should copy www to inst/app/www if it exists
  • We should read the DESCRIPTION file if ever it exists (and don't override it: it seems like a common thing inside the shiny example repo: https://github.com/rstudio/shiny-examples/tree/master/001-hello)
  • Leave a message about splitting the UI / Server into the related functions
  • usethis::use_buildignore("app.R") if it exists.

Also, this should be documented clearly (a vignette ?)

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

No branches or pull requests

1 participant