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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't include CRAN install instructions to start #1507

Merged
merged 15 commits into from Oct 7, 2021
Merged

Conversation

malcolmbarrett
Copy link
Collaborator

@malcolmbarrett malcolmbarrett commented Oct 5, 2021

This closes #1451 by tweaking the default README instructions. We don't need to add a release bullet point reminding to tweak install instructions because, as it turns out, there's been one for a few years already (馃槅)

The downside of this simple change is that, if the repo is not on GitHub, nothing gets added to the installation section, and it will be just the header. We could only include the header if already on GH or we could leave it empty as a prompt, perhaps with a poke with ui_todo()

I think there's a lot of people (馃) who create the README prior to putting on GH, as well.

@jennybc
Copy link
Member

jennybc commented Oct 7, 2021

The downside of this simple change is that, if the repo is not on GitHub, nothing gets added to the installation section, and it will be just the header. We could only include the header if already on GH or we could leave it empty as a prompt, perhaps with a poke with ui_todo()

I've pushed a change that does insert some sort of placeholder in the "no GitHub (yet)" case.

What you do think @malcolmbarrett? We could still combine this with a nudge to populate this section via ui_todo().

@@ -75,6 +75,7 @@ release_checklist <- function(version, on_cran) {
"",

todo("`usethis::use_cran_comments()`"),
todo("Update (aspirational) install instructions in README"),
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 one should revise the README to be (or at least include) install.packages() at the time of CRAN submission, because hopefully that will soon be true and that's what you want baked into the README on CRAN.

@malcolmbarrett
Copy link
Collaborator Author

ruh roh, added it at the same time! I'll revoke mine

@malcolmbarrett
Copy link
Collaborator Author

Oh wait, that's the checklist item. That makes sense

@jennybc
Copy link
Member

jennybc commented Oct 7, 2021

I brought use_readme() along. I think this is ready to merge @malcolmbarrett but would appreciate a second opinion.

Ideally it would be supported in expect_snapshot_file(), but this workaround serves for now.
@jennybc jennybc merged commit a0b2084 into master Oct 7, 2021
@jennybc jennybc deleted the adjust_readme_install branch October 7, 2021 20:59
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.

Default README mentioning installing from CRAN is potentially confusing for new package authors
2 participants