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

Some tweaks + suggestions #139

Merged
merged 1 commit into from
Oct 20, 2023
Merged

Some tweaks + suggestions #139

merged 1 commit into from
Oct 20, 2023

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Oct 20, 2023

  • I found it confusing that there was keyring as the package name and keyring as the thing where secrets are stored. Actually maybe add a sentence saying "Keyrings are the objects where the keyring package stores secrets" or something similar?
  • I removed the Latin "e.g." in favor of the more readable "for instance".
  • I wrote "environment variables" in full.

I wonder whether there could be two links

Second to last suggestion, could there be a comparison of workflows, like

## Without using keyring

1. The user set an environment variable `MYSECRET="super-secret"` 
    - in a project-specific `.Renviron`, hopefully gitignored;
    - or in the user `.Renviron`;
    - or types `Sys.setenv(MYSECRET="super-secret")` in the console, hopefully protecting `.Rhistory`.
1. The package retrieves the secret with `Sys.getenv("MYSECRET")`.

## With keyring

1. The user stores the secret using `keyring::key_set("MYSECRET")`, typing interactively so nothing is recorded in `.Rhistory`.
1. The user sets an environment variable with `Sys.setenv(MYSECRET = keyring::key_get("MYSECRET"))` in a script for instance.
1. The package retrieves the secret with `Sys.getenv("MYSECRET")`.

Last suggestion, for a package developer, most often keyring is more something to document than to use, correct? In a package using secrets you'd recommend a certain workflow or point user to keyring, but not use it inside your code?

@maelle
Copy link
Contributor Author

maelle commented Oct 20, 2023

Further thought after updating my local GitHub PAT using gitcreds: maybe there could be a mention of gitcreds, to underline the parallels between the two? I used to store my local GitHub PAT in .Renviron.

@gaborcsardi gaborcsardi merged commit 6c350c3 into r-lib:main Oct 20, 2023
12 checks passed
@gaborcsardi
Copy link
Member

Thanks!

@gaborcsardi
Copy link
Member

In general I am wary of putting too many links into the documentation, these links tend to go stale. It is a lot of work to make sure they point to the right place. If people don't know what environment variables are, they can google "environment variables" and probably get a good link. I think extensive links and workflow suggestions are better suited to blog posts, or tutorials, and not so much for the reference manual or the README.

@maelle
Copy link
Contributor Author

maelle commented Oct 20, 2023

For info @gaborcsardi I came here via r-lib/httr2#346

gaborcsardi pushed a commit that referenced this pull request Oct 20, 2023
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