feat(pull): add bypass-rules opt-in and expose scheduled auto-merge via API - #38404
feat(pull): add bypass-rules opt-in and expose scheduled auto-merge via API#38404bircni wants to merge 14 commits into
Conversation
c850ce7 to
bf49b39
Compare
…bypass # Conflicts: # web_src/js/components/PullRequestMergeForm.vue
|
TBH, the checkbox UI/UX seems strange. Personally I am used to the old UI. I prefer to keep the UI concise, avoid unnecessary interactive components when end user doesn't really need it. That's just my opinion, just FYI. If most people like the new UI, I am neutral |
Or, if the "bypass" decision is not made by a checkbox, but by a secondary "toggle link/button" like "Switch to auto merge / Switch to force merge", maybe the UI would look better. |
|
Thanks @wxiaoguang for the response - i will take a look at it |
|
@wxiaoguang changed it to this: |
|
@silverwind what do you think about the new UI?
|
|
ping @silverwind |
|
Looks ok but I would do something to align the button colors of the merge box with github, they only have two states, uncolored and green. Those red/blue buttons confuse me and their meaning is not really conveyed. Whether the primary action is green or primary (blue) I don't really care but I guess for consistency it should remain blue. |
|
I think we should keep the red color if you are doing something "bad" (bypass merging) |
|
@silverwind @wxiaoguang ping |
Is silverwind happy with it now? |
|
I'd still prefer the same ui as GitHub has with the checkbox but I am also fine with this solution |
|
I guess I need to take a look at this UI myself. This stray red text looks confusing. The whole merge box is full of confusing UI, I prefer a re-alignment with GitHub. |
…bypass # Conflicts: # models/pull/automerge.go # services/convert/pull.go
| SvgIconHTML: svg.RenderHTML("octicon-dot-fill", 16, "tw-text-text-light"), | ||
| InfoHTML: info, | ||
| ListItems: util.OptionalArg(optItems), | ||
| ExtraClass: "tw-pl-6 tw-text-text-light", |
There was a problem hiding this comment.
I don't think it is a right design.
The pullMergeBoxInfoItemCollection is designed to be a flat list. Now, you just added the hacky patches to make it "indented" and "grouped".
It is very fragile.
I believe that "don't try to teach a duck to bark to make it become a dog".
If you need a new layout, design a proper data structure for it. Complex changes can be in a separate and dedicated PR.
There was a problem hiding this comment.
It seems working because at the moment all callers to AddErrorItem are on infoProtectionBlockers, then the design is abused by this PR.
But indeed AddInfoItem / AddErrorItem are similar functions, the layout will break if AddInfoItem / AddErrorItem are both called for the same section.
We should avoid the unclear and fragile behaviors when designing the functions and methods. Do not bring surprises to developers.



Reworks the pull request merge box to match GitHub's behaviour and surfaces auto-merge state on the API.
Merge box
force_mergeandmerge_when_checks_succeedare never both sent.API
PullRequestresponses gain a GitHub-compatibleauto_mergeobject (enabled_by,merge_method,commit_title,commit_message), ornullwhen not scheduled — on both the single-PR and list endpoints (batched to avoid N+1). Swagger regenerated.Screenshots