Skip to content
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

[#104] Support setting the remote during lab mr checkout #120

Merged
merged 2 commits into from
Mar 18, 2018

Conversation

adamryman
Copy link
Contributor

@adamryman adamryman commented Mar 12, 2018

Works as follows:

lab mr checkout -t !mrID

When -t or --track is specified the following happens.

  • Check for remote by the username if the MR author
    • If the remote does not exist, add it as `mr.Author.Username
  • Instead of fetching to the configured branch, fetch to refs/remotes/mr.Author.Username/mr.sourceBranch (fetchToRef)
  • Create a local branch starting from fetchToRef
  • Checkout that branch

Test on the way.

Closes #104

@adamryman adamryman added this to the 0.10.0 milestone Mar 12, 2018
@adamryman adamryman requested a review from zaquestion March 12, 2018 01:52
@codecov
Copy link

codecov bot commented Mar 12, 2018

Codecov Report

Merging #120 into master will decrease coverage by 0.86%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
- Coverage   62.12%   61.25%   -0.87%     
==========================================
  Files          32       32              
  Lines        1270     1288      +18     
==========================================
  Hits          789      789              
- Misses        317      333      +16     
- Partials      164      166       +2
Impacted Files Coverage Δ
internal/gitlab/gitlab.go 49.7% <0%> (-2.15%) ⬇️
cmd/mrCheckout.go 40.54% <38.09%> (-17.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b212da7...6584ce6. Read the comment docs.

Works as follows:
```
lab mr checkout -t !mrID
```

When `-t` or `--track` is specified the following happens:

- Check for remote by the username of the MR author
    - If the remote does not exist, add it add mr.Author.Username
- Instead of fetching to the configured branch, fetch to
`refs/remotes/mr.Author.Username/mr.sourceBranch` (`fetchToRef`)
- Create a local branch starting from `fetchToRef`
- Checkout that branch
@adamryman adamryman force-pushed the mrCheckoutTracking branch from f1320fa to aff5e24 Compare March 12, 2018 01:57
fetchToRef = fmt.Sprintf("refs/remotes/%s/%s", mr.Author.Username, mr.SourceBranch)
}

// https://docs.gitlab.com/ee/user/project/merge_requests/#checkout-merge-requests-locally
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes, looks like I put that there 😊

if mrCheckoutCfg.track {
// Create configured branch with tracking from fetchToRef
// git branch --flags <branchname> [<start-point>]
gitb := git.New("branch", "--track", mrCheckoutCfg.branch, fetchToRef)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the --track functionality can be used directly with git checkout. Is it worth combining the 2 commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would still need to check out this branch, as git checkout --track -b will track the new branch off of the previous branch. So if we use git checkout --track we need two git checkout commands.

I prefer this way personally.

@@ -81,6 +81,18 @@ var (
localProjects map[string]*gitlab.Project = make(map[string]*gitlab.Project)
)

// GetProject looks up a Gitlab project by ID.
func GetProject(projectID int) (*gitlab.Project, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just update FindProject to accept an interface{} and switch for the type. go-gitlab will accept the interface form as well

Copy link
Contributor Author

@adamryman adamryman Mar 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After putting 15min into this. I request some help. The problem is resolving the whole localProjects map.

How about I add a TODO? Maybe an issue as well? Or we could pair on it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After chatting with you more (in person) this is fine as is.

}

// Check out branch
gitc := git.New("checkout", mrCheckoutCfg.branch)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of a bad pattern on my part is looks like. These can really just be combined like

err := git.New("checkout", mrCheckoutCfg.branch).Run()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooooooooooooooooooooooooooooo. I like that 👍

@zaquestion
Copy link
Owner

This just needs tests to merge.

@zaquestion
Copy link
Owner

@adamryman you owe tests! :D

@zaquestion zaquestion merged commit efdf23c into zaquestion:master Mar 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants