Conversation
mislav
left a comment
There was a problem hiding this comment.
Nice start!
Something I would like us to consider is potentially designing Context as an interface, and having other methods accept that interface when querying the context? That way, we could supply a fake context with mocked values during tests.
Still not clear on how exactly it would look like, but it might get clearer once we start writing more unit tests and experience the pain of isolating them from fs lookups.
| return "", err | ||
| } | ||
|
|
||
| return strings.Replace(currentBranch, "refs/heads/", "", 1), nil |
There was a problem hiding this comment.
I wonder if there would be some value in passing around the branch as a Branch object instead of a string? The String() method of the Branch object could then return the name of that branch without the refs/heads/ prefix.
context/context.go
Outdated
| return strings.Replace(currentBranch, "refs/heads/", "", 1), nil | ||
| } | ||
|
|
||
| func CurrentRepository() (GitRepository, error) { |
There was a problem hiding this comment.
Perhaps this method name could be less ambiguous re: whether it refers to the "repository" as local git repository or "repository" as a GitHub Repository object?
There was a problem hiding this comment.
yeah -- i'm still stuck on what a good implementation for keeping git and github repositories separate would be.
Honestly, I think I'm starting from too general a place here and should step back and work on something else...
There was a problem hiding this comment.
That makes sense! Maybe a better place to start would be top-down; i.e. start with a real command that needs to query current git branch, fetch some git config, and potentially fetch info about the current GitHub repo. How would we invoke this command in tests and stub out the current branch info? Also, if information like current branch or the list of git remotes is accessed more than one time per invocation of a command, do we perform any duplicate lookups? How to cache the results of those lookups instead.
Alternatively to stubbing in memory: the test could also prepare a real, isolated git repo just for the test, setup the current branch and some GitHub remotes in it, and cd into that repo for the duration of the test. However, I found out that this slows down tests significantly.
Co-Authored-By: Mislav Marohnić <mislav@github.com>
|
Should this target |
|
I'm re-doing this work against master and will either make a new PR or just force push when that's ready. |
Added flags: --settings, --projects, and --wiki
This WIP PR starts a
contextpackage that is intended to be called from commands to get information about the current state of a user, their config, their current repo, and their github account.