search

r-lib / gitcreds

Query git credentials from R
https://gitcreds.r-lib.org/
Other
28 stars 11 forks source link

is gitcreds_set() a little too aggressive with errors? #25

Closed malcolmbarrett closed 3 years ago

malcolmbarrett commented 3 years ago

I was exploring this (very cool) package today and was surprised that, when I ran gitcreds::gitcreds_set() and kept my existing credentials, it threw an error. I expected it to end the interaction silently without a warning or error (likely something invisible(). The main reason I expected that was because selecting the "no" path doesn't seem like an error to me. I think I've also been trained to expect that behavior from usethis and devtools

gaborcsardi commented 3 years ago

We consider gitcreds to be an infrastructure package that you can build on, and the intent here it to communicate it back to upstream code that the user aborted setting new credentials. There are other ways to do this, but doing it via a classed error has some advantages.

If you want to build on gitcreds, you can catch this (classed) error, and implement a different UI.

When you use gitcreds directly interactively, then it probably does not matter too much if it is an error or not.

malcolmbarrett commented 3 years ago

Ah, that's pretty convincing. I'll keep that in mind, as very likely I would be using it in a usethis-like package. Thanks for clarifying!

malcolmbarrett commented 3 years ago

In fact, my opinion has made a total 180, and I now think this is actually really elegant 😄

gaborcsardi commented 3 years ago

Cool. :) I wasn't entirely sure about this either at first, but I am more sure now. Nevertheless, feedback about pain points is welcome. Btw. in case you haven't seen it, there is also a vignette for package authors: https://gitcreds.r-lib.org/articles/package.html

malcolmbarrett commented 3 years ago

Thank you, this is great! Yes, I filed it exactly because I thought it might be useful to know my user experience. I'm glad I did, as this discussion gives me a helpful way to think about using it in a package. Thanks again, Gabor!