Skip to content

Internal git fixes#568

Merged
bmeneg merged 2 commits intozaquestion:masterfrom
atenart:internal-git-fixes
Jan 25, 2021
Merged

Internal git fixes#568
bmeneg merged 2 commits intozaquestion:masterfrom
atenart:internal-git-fixes

Conversation

@atenart
Copy link
Copy Markdown
Contributor

@atenart atenart commented Jan 19, 2021

Hi,

This PR fixes one error message in internal/git and the UpstreamBranch function which has a too strict expectation on the local remote names.

Thanks!

@prarit
Copy link
Copy Markdown
Collaborator

prarit commented Jan 21, 2021

Hi, due to no fault of yours the lab test code was broken. I have fixed the tests in #575 and #573 . Can you please rebase to see if your build will past testing? My sincere apologies for the inconvenience.

@atenart
Copy link
Copy Markdown
Contributor Author

atenart commented Jan 22, 2021

@prarit Sure, no problem, I'll rebase and update this PR.

@atenart atenart force-pushed the internal-git-fixes branch 2 times, most recently from dd99f46 to 8a2be40 Compare January 22, 2021 08:36
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 22, 2021

Codecov Report

Merging #568 (ee99ea6) into master (910234a) will decrease coverage by 0.03%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #568      +/-   ##
==========================================
- Coverage   58.11%   58.07%   -0.04%     
==========================================
  Files          64       64              
  Lines        4219     4215       -4     
==========================================
- Hits         2452     2448       -4     
+ Misses       1529     1528       -1     
- Partials      238      239       +1     
Impacted Files Coverage Δ
internal/git/git.go 55.08% <66.66%> (+0.10%) ⬆️
cmd/mr_create.go 75.50% <0.00%> (-1.00%) ⬇️

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 910234a...ee99ea6. Read the comment docs.

Comment thread internal/git/git.go
Comment thread internal/git/git.go Outdated
Comment thread internal/git/git.go Outdated
UpstreamBranch is assuming the value returned by `git rev-parse` match
'<remote name>/<branch name>' and split the string on the first / to
retrieve the remote branch name. That only works when the remote name
does not have slashes in its name, otherwise the returned branch name is
invalid and not pointing to a git valid object:

- If the remote is 'origin', the rev-parse returned value would be like
  'origin/branch/name' and UpstreamBranch would return 'branch/name'.

- But if the remote is 'gitlab/origin', the rev-parse returned value
  would be like 'gitlab/origin/branch/name' and UpstreamBranch would
  return 'origin/branch/name'.

`git rev-parse --abbrev-ref @{upstream}` constructs its value by
appending branch.<name>.remote and branch.<name>.merge (and removing
'refs/heads/' from the merge part) according to its man page.

What we really want here is branch.<name>.merge (without 'refs/heads/'),
not @{upstream}. This patch is implementing this.

Signed-off-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Antoine Tenart <atenart@kernel.org>
@atenart atenart force-pushed the internal-git-fixes branch from 8a2be40 to ee99ea6 Compare January 22, 2021 21:21
Copy link
Copy Markdown
Collaborator

@bmeneg bmeneg left a comment

Choose a reason for hiding this comment

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

New changes LGTM.
Thanks @atenart

@bmeneg bmeneg merged commit 745ecf0 into zaquestion:master Jan 25, 2021
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.

4 participants