Conversation
ttaylorr
left a comment
There was a problem hiding this comment.
Looking good, and mad ⚡️'s for tackling this. I think we should have some discussion about what to call this new type, but otherwise 👍
config/config.go
Outdated
| return c.endpointConfig().GitProtocol() | ||
| } | ||
|
|
||
| func (c *Configuration) endpointConfig() *endpoint.Config { |
There was a problem hiding this comment.
I really dig this. It's too bad that we have to keep something like it around in *config.Configuration, but it's nice that we can track all of the uses of it by checking out the callers of this function.
endpoint/config.go
Outdated
|
|
||
| const defaultRemote = "origin" | ||
|
|
||
| type Config struct { |
There was a problem hiding this comment.
I'm still not sold on having a per-package *Config type. I think that splitting out and removing stuff from the *config.Configuration type is really good, but I think that we should go one step further and figure out what an appropriate abstraction is.
What do you think about *endpoint.Finder?
There was a problem hiding this comment.
Yeah, I dig that. Still not sure if this belongs in here or in api, though.
Experiment in extracting the endpoint and url alias stuff from
configto a newendpointpackage. I'm starting to think it belongs inapiinstead though.