KentonWhite / ProjectTemplate

A template utility for R projects that provides a skeletal project.
http://projecttemplate.net
GNU General Public License v3.0
623 stars 159 forks source link

Change /doc to /docs #177

Closed Hugovdberg closed 7 years ago

Hugovdberg commented 7 years ago

Recently GitHub made publishing a website on GitHub Pages a lot easier: see this blog post. There are two options to set the Pages root in the master branch, either use the root directory or the docs folder. By renaming the doc folder to docs in new projects we facilitate the latter option.

As the doc folder isn't included in the default workflow and mainly for convenience as far as I can see it should not have a major impact on our project.

connectedblue commented 7 years ago

Nice - I hadn't seen that post. I keep meaning to learn jekyll etc, but this looks super easy.

Agreed that your proposed change is relatively minor. There would also need to be a change to the exclude variable in create.project so that minimal projects keep working correctly.

I would also suggest putting some detection code in migrate.project to change the directory name in existing projects, with a message to the user to explain that this has been done.

Hugovdberg commented 7 years ago

I'm not sure about changing existing projects, existing links to the doc folder might break and cannot be changed automatically. I would suggest to create a message pointing to the blog post to notify the user of the possibility, or at the very least prompt the user whether the folder should be renamed.

connectedblue commented 7 years ago

Yes, there are edge cases either way. The purpose of migrate.project is to bring a previously created project up to date with the latest version (as if create.project has been run again).

If the doc directory remains after migration, then migrate.project loses its integrity for all users just because some might have broken links.

A user prompt would be a nice middle ground.

KentonWhite commented 7 years ago

I agree this would be a nice change. Change existing projects through the migrate.project function. I would not erase or rename the doc directory with migrate.project but instead simply add the docs directory to existing projects. This prevents those projects from breaking. The user can then delete the doc directory if they desire. New projects should only have the docs directory.

connectedblue commented 7 years ago

@KentonWhite neat solution. Best of both worlds - forward and backward compatibility :)

Hugovdberg commented 7 years ago

Simply creating the docs directory is indeed a better option, but I think there should be a message that the new docs isn't just a typo (for the people who missed the update). So tasks to do:

Missing anything?

connectedblue commented 7 years ago

also, in create.project() around line 53, the check for minimal needs updating - the exclude variable needs updating to "docs".

You could also update the README in the new docs directory to show how to integrate easily with github pages and the blog post link.

Hugovdberg commented 7 years ago

Added your remarks to the checklist.

On a sidenote, shouldn't we rename the website/ folder in this repository to docs/ as well or will that conflict with building the package?

KentonWhite commented 7 years ago

The website folder should not be changed at this time. It is used to build the ProjectTemplate.net site and uses and older workflow. I should update the workflow so it is compatible with github pages.

Hugovdberg commented 7 years ago

migrate.project is a difficult case anyway I think, because in my opinion it should obey the minimal project option if it was true in the call to create.project. Since the other folders excluded for a minimal install aren't created during migration either we shouldn't add just docs. I will add a new issue for this as I think we should carefully consider how projects are created and migrated, including sensible merge strategies.

connectedblue commented 7 years ago

Yes, very good point.

Maybe migrate.project should check for doc and if it exists, create a docs. A bit hacky, but should get the job done. Meanwhile we should think more carefully as your suggest.

Who would have thought it was so difficult to add an s?

Hugovdberg commented 7 years ago

Pull request #180 resolves this, all further discussion on implementation should be moved there. Closing this issue.