sindresorhus/refined-github

wait-for-build doesn't work if merging is disabled before build #3940

C-Higgins posted onGitHub

If the repo is configured to disallow merging until builds pass, the wait-for-build option will not appear, although this seems like it should be part of the functionality. The problem is in this code:

        // The user cannot merge
        () => !select.exists('[data-details-container=".js-merge-pr"]:not(:disabled)')

The merge button might be disabled because builds haven't passed yet, not because the user doesn't have merge permissions in general.


cc @jack1142 since he may be more familiar with the code (https://github.com/sindresorhus/refined-github/pull/3893)

posted by fregante about 4 years ago

This sounds more like an enhancement proposal/feature request rather than a bug. I've actually never noticed that this is a thing as I have admin perms on the repos I'm most active on and we don't have "Include administrators" selected in the branch protection settings so at the very least I can always admin merge.

Anyway, I'm thinking that it isn't strictly a bug as I think the wait-for-build feature is more directed at people who want to wait for the build even though they can already merge (because the branch protections are met or because they can admin merge). IMO the issue that is shown here can already be (mostly) solved by the newly introduced auto-merge feature from GitHub. The difference between GitHub's auto-merge and wait-for-build feature is that RGH's also waits for optional checks, so there is technically a gap that RGH could still fill, but I'm not sure whether there are many people who wouldn't already be satisfied by GitHub's auto-merge for that and as any feature, it comes with maintenance burden.

But if maintainers wanted to include this in RGH, RGH would have to remove the "disabled" state of all the merge buttons so that they are clickable (it seems that clicking on them still shows you to the merge form and allows to perform merge so that wouldn't be much of a problem I'm guessing). There are still other branch protections like approval count, however, and they must be met before the merge can happen, and that would more than likely complicate the whole logic.

cc @jack1142 since he may be more familiar with the code (#3893)

I'm probably not that familiar with the code of that feature (not more than I needed to make the PR anyway) but figured I might use the occasion to send my thoughts 😄

posted by jack1142 about 4 years ago

I think this is a duplicate of of https://github.com/sindresorhus/refined-github/issues/1325

If this is what you see, then it is:

image

posted by fregante about 4 years ago

the issue that is shown here can already be (mostly) solved by the newly introduced auto-merge feature from GitHub.

Yep you're right, I did not know that feature existed. This feature/fix is not necessary then.

posted by C-Higgins about 4 years ago

Fund this Issue

$0.00
Funded

Pull requests