Add repository.ParseWithHost #41
Conversation
| if host == "" { | ||
| host = "github.com" | ||
| host := "github.com" | ||
| cfg, err := config.Load() |
There was a problem hiding this comment.
This seems correct for now, but ultimately I find it surprising that Parse() implicitly loads config. I think an ideal API might be one where a repository "parser" is derived from a config object, so that their relationship is denoted explicitly.
There was a problem hiding this comment.
Yeah I agree it is a bit odd. go-gh is also targeting extension authors, I would think that they don't necessarily want to concern themselves with loading up the config and determining the correct host. We also do this when initializing the various API clients. As long as there are alternatives for advanced users, that don't want automatic resolution such as repository.ParseWithHost, I am okay with having these easy to use alternatives.
This PR adds a
repository.ParseWithHostfunction that is necessary forghto usego-ghfor parsing repository strings. Additionally, it changesrepository.Parseto now look at the config file for a viable host instead of just looking atGH_HOSTbefore defaulting togithub.com. This new behavior also matches the current functionality ingh.cc cli/cli#5560
corresponds to cli/cli#5655