-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[Branch View] manual merge tag #7979
Conversation
Imho this should be checked on push and saved to pr |
I dont understand? What do you mean? |
Nevermind, it's late I thought it is about PRs manual merge displaying :) |
do it the short way ... #7976 (comment) |
Codecov Report
@@ Coverage Diff @@
## master #7979 +/- ##
==========================================
+ Coverage 41.79% 41.81% +0.01%
==========================================
Files 497 497
Lines 65603 65608 +5
==========================================
+ Hits 27420 27434 +14
+ Misses 34667 34659 -8
+ Partials 3516 3515 -1
Continue to review full report at Codecov.
|
@lafriks can you review it? |
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.
Just to ask, is this also going to indicate a branch is merged if the branch was just created and pushed from the master branch?
It does. -> #7976 (comment) |
IMHO, having those branches that have no commits ahead of the master being called merged seems a bit weird as it could also mean that the branch was just created. That may cause some confusion. |
we can check if it is at least one commit behind the default one to avoid souch or if it doesnt use to mouch cpu time lookup the merge commit |
We also could detect if the second commit of the branch is not the second one of the default branch. |
nice idear this would save lookup time and works with a lot of constructs ...
|
to test if divergence.Behind != 0 should have the same impact? because if branch is 0 Ahead the only way bouth parent commits are the same is that it is also 0 commits behind ... |
if: * no commits ahead default branch * not same as default branch
1be6cc9
to
8f80f3e
Compare
I cleanup the 22 commits so it is easy to understand - the actual diff is very smal ... |
thx now its ready to merge 🚀 EDIT: there is a merge frees for now? (release) |
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.
@lunny changed it ...
@@ -203,10 +204,16 @@ func loadBranches(ctx *context.Context) []*Branch { | |||
} | |||
} | |||
|
|||
isMerged := false | |||
if divergence.Ahead == 0 && divergence.Behind > 0 && ctx.Repo.Repository.DefaultBranch != branchName { |
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.
And I thought it again, maybe divergence.Behind > 0
should be divergence.Behind >= 0
? Consider a branch just merged to master
and no new commits pushed to master.
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.
but this also let branches who are clones of default-branch show up as "merged":
git checkout -b testbranch origin/master
git push -u origin testbranch
Close thiss issue because of brainstorm with lunny
|
(#7976) Show Merge Tag if it is merged manualy
UI: