Find DefaultPRHead based on branch and SHA #514

Merged
6543 merged 4 commits from 6543/tea:enhance_find-remote-of-branch into main 2 months ago
6543 commented 2 months ago
Collaborator

enhance

enhance
6543 added this to the v0.10.0 milestone 2 months ago
6543 added the kind/enhancement label 2 months ago
6543 added 1 commit 2 months ago
continuous-integration/drone/pr Build is passing Details
b83875ae41
Find DefaultPRHead based on branch and SHA
6543 requested review from Maintainers 2 months ago
noerw commented 2 months ago
Collaborator

Can you describe which case is enhanced with this?

This might give wrong / ambiguous results, I think that was the reason I did not pass the sha in:
Did you test what happens when you pass a sha to TeaFindBranchRemote() that is on multiple branches? Which branch / remote will be selected? Or can't this case occur?

Can you describe which case is enhanced with this? This might give wrong / ambiguous results, I think that was the reason I did not pass the sha in: Did you test what happens when you pass a sha to `TeaFindBranchRemote()` that is on multiple branches? Which branch / remote will be selected? Or can't this case occur?
6543 added 1 commit 2 months ago
continuous-integration/drone/pr Build is passing Details
757bd0cb72
TeaFindBranchRemote() make specific priority order
6543 commented 2 months ago
Poster
Collaborator

@noerw added specific priority order

@noerw added specific priority order
strk approved these changes 2 months ago
strk left a comment
Collaborator

Approving, but no testcase here ?

Approving, but no testcase here ?
6543 commented 2 months ago
Poster
Collaborator

I might could add unit test for this func ...

I might could add unit test for this func ...
6543 added 1 commit 2 months ago
6543 added 1 commit 2 months ago
noerw commented 2 months ago
Collaborator

I can't observe a difference in behaviour. Again, can you clarify what case is better handled with this? When the local branch is named differently than the remote branch?

I can't observe a difference in behaviour. Again, can you clarify what case is better handled with this? When the local branch is named differently than the remote branch?
6543 commented 2 months ago
Poster
Collaborator

I do work with multiple remotes so I do see difference in behavior.

e.g. this pull was created with tea with this patch...

I do work with multiple remotes so I do see difference in behavior. e.g. this pull was created with tea with this patch...
6543 commented 2 months ago
Poster
Collaborator

... but let's make it wip and I see how i can add tests

... but let's make it wip and I see how i can add tests
6543 added the status/wip label 2 months ago
noerw approved these changes 2 months ago
noerw commented 2 months ago
Collaborator

I still don't get the exact context where this helps, but it doesn't seem to break anything :)

I still don't get the exact context where this helps, but it doesn't seem to break anything :)
noerw requested changes 2 months ago
noerw left a comment
Collaborator

never mind, this breaks the default values for me after further testing:

ignore me, i'm tired. I tested with detached head, that's not supposed to set default values.

~~never mind, this breaks the default values for me after further testing:~~ ignore me, i'm tired. I tested with detached head, that's not supposed to set default values.
noerw approved these changes 2 months ago
6543 removed the status/wip label 2 months ago
6543 merged commit 54b535a527 into main 2 months ago
6543 deleted branch enhance_find-remote-of-branch 2 months ago

Reviewers

strk approved these changes 2 months ago
noerw approved these changes 2 months ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 54b535a527.
Sign in to join this conversation.
Loading…
There is no content yet.