-
Notifications
You must be signed in to change notification settings - Fork 502
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
Refactor the rest of the git methods #937
Refactor the rest of the git methods #937
Conversation
- We now checkout the target branch after clone/open - Added cleanup defer for repo as well - The working directory will now contain a link to the repo to make the go env rewrite work - `MergeBase` is now a method of `Repo` - `RevParse` and `RevParseShort` are now methods of `Repo` Signed-off-by: Sascha Grunert <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saschagrunert -- You're quick! Just one comment on whether or not there's a typo in here.
if nomock { | ||
log.Printf("nomock mode (from within ff)\n") | ||
gitRoot := fmt.Sprintf("%s/src/k8s.io", workingDir) | ||
if err := os.MkdirAll(gitRoot, 0o755); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this (0o775
) a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is new in go 1.13, see https://golang.org/ref/spec#Integer_literals
I assume this is fine since we're only supporting this version, right? 😇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhhh, that's cool! TIL :)
...and yes, onwards to go1.13!
Thanks @saschagrunert! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justaugustus, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
to make the go env rewrite work
MergeBase
is now a method ofRepo
RevParse
andRevParseShort
are now methods ofRepo
SynchRepo
function is now obsolete since auth/clone/open works viaRepo
I think as a next step we could refactor the rest of the
TODO
s inff.go
😇