Skip to content
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

Add auto-merge feature for GitHub and GitLab #903

Merged
merged 15 commits into from
Mar 16, 2022
Merged

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Feb 28, 2022

CML is a great tool! We're using it to automatically pull in new data into our repository daily. Currently, we're reviewing + merging every PR manually. We'd like to automate this, and would like to use Gitlab's auto-merge feature to do so. This PR is a first iteration on supporting this. We'll test internally, and if it works, possibly extend the feature to the GitHub driver.

@DavidGOrtega
Copy link
Contributor

👋 @Skn0tt thanks for the contribution.
We have already talked about this auto-merge feature. Let's review it 😃

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Feb 28, 2022

Note: this feature is also supported on GitHub and Bitbucket, although I have never user the latter.

@Skn0tt Skn0tt temporarily deployed to external March 1, 2022 07:58 Inactive
src/drivers/gitlab.js Outdated Show resolved Hide resolved
@Skn0tt
Copy link
Contributor Author

Skn0tt commented Mar 1, 2022

Note: this feature is also supported on GitHub and Bitbucket, although I have never user the latter.

@0x2b3bfa0 do you know of any way to programmatically enable auto-merge on GitHub? couldn't find anything in their API Docs

@0x2b3bfa0
Copy link
Member

@0x2b3bfa0 do you know of any way to programmatically enable auto-merge on GitHub?

It's only available through GraphQL as far as I know. See enablePullRequestAutoMerge and EnablePullRequestAutoMergeInput for more information.

@0x2b3bfa0
Copy link
Member

Would probably imply using @octokit/graphql instead of / apart from @octokit/rest

@DavidGOrtega
Copy link
Contributor

Note: this feature is also supported on GitHub and Bitbucket, although I have never user the latter.

@0x2b3bfa0 do you know of any way to programmatically enable auto-merge on GitHub? couldn't find anything in their API Docs

The equivalent of what you do using octokit

@0x2b3bfa0

This comment was marked as off-topic.

@DavidGOrtega
Copy link
Contributor

DavidGOrtega commented Mar 1, 2022

@0x2b3bfa0

But @Skn0tt is doing a synthetic "auto-merge" feature in GL using

const endpoint = `/projects/${projectPath}/merge_requests/${mergeRequestId}/merge`;

The equivalent is just merge as it is doing right now?
Maybe Im missing something?

@0x2b3bfa0
Copy link
Member

Maybe Im missing something?

This pull request makes use of merge_when_pipeline_succeeds (documentation) which is the functional equivalent of GitHub auto–merge.

@DavidGOrtega
Copy link
Contributor

DavidGOrtega commented Mar 1, 2022

@0x2b3bfa0 Are they the same? It seems that in GL merges if pipeline succeed I cant find that specs in the GH docs, maybe they are expecting to to additionally setup branch protection?

@DavidGOrtega
Copy link
Contributor

DavidGOrtega commented Mar 1, 2022

status checks have passed.

@0x2b3bfa0 so Am I right??

The issue that I see is that you must to upgrade then to use the feature?

image

@DavidGOrtega
Copy link
Contributor

We should do a trial to see if it works as expected beforehand

@DavidGOrtega
Copy link
Contributor

If not maybe just synthetic PR may work taking into account that merge would be the last op without having fail at least the current step in the workflow (not yet a successfully workflow)

@casperdcl casperdcl mentioned this pull request Mar 1, 2022
@casperdcl casperdcl temporarily deployed to external March 9, 2022 21:59 Inactive
casperdcl
casperdcl previously approved these changes Mar 9, 2022
@casperdcl casperdcl added ci-github ci-gitlab cml-pr Subcommand enhancement New feature or request labels Mar 9, 2022
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Should we keep JSDoc comments? There is no precedent in our code base, although it's a tempting improvement.

src/drivers/github.js Outdated Show resolved Hide resolved
@DavidGOrtega
Copy link
Contributor

Looks good to me! Should we keep JSDoc comments? There is no precedent in our code base, although it's a tempting improvement.

Good question.
If we accept it we should follow up a technical debt issue to fulfil the rest

@0x2b3bfa0
Copy link
Member

@Skn0tt Skn0tt temporarily deployed to external March 15, 2022 09:46 Inactive
Copy link
Contributor

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

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

lgtm - test failing due to unrelated #914

package.json Show resolved Hide resolved
@Skn0tt Skn0tt temporarily deployed to external March 16, 2022 12:55 Inactive
Copy link
Contributor

@DavidGOrtega DavidGOrtega left a comment

Choose a reason for hiding this comment

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

👍

@DavidGOrtega
Copy link
Contributor

opens #916

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to external March 16, 2022 13:17 Inactive
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

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

:shipit:

@0x2b3bfa0 0x2b3bfa0 changed the title feat: add auto-merge feature for gitlab Add auto-merge feature for GitHub and GitLab Mar 16, 2022
@casperdcl casperdcl merged commit c03eeee into iterative:master Mar 16, 2022
@casperdcl
Copy link
Contributor

casperdcl commented Mar 16, 2022

Thanks @Skn0tt! Will be released soon (v0.12.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-github ci-gitlab cml-pr Subcommand enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants