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

Fix automerge action #10055

Closed
wants to merge 13 commits into from
Closed

Fix automerge action #10055

wants to merge 13 commits into from

Conversation

koppor
Copy link
Member

@koppor koppor commented Jul 3, 2023

We are having bots creating Pull Requests. Example: #10054. We have an action for auto-merging these PRs if they are OK. The bot, however, does not work currently. This PR tries to fix it.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

Quoting the README of the action

A pull request is considered ready when:

  1. the required number of review approvals has been given (if enabled in the branch protection rules) and
  2. the required checks have passed (if enabled in the branch protection rules) and
  3. the pull request is up to date (if enabled in the branch protection rules)

I overwrite the number of required reviews to 0, because "we" automatically approve the merge if all tests are green.

When the pull request is ready, it will automatically be merged. The action
will only wait for status checks that are marked as required in the branch
protection rules

@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

Current output:

Run pascalgn/automerge-action@v0.15.6
2023-07-03T09:38:46.738Z INFO  Event name: pull_request
2023-07-03T09:38:47.227Z INFO  Skipping PR update, required label missing: automerge
2023-07-03T09:38:47.228Z INFO  Skipping PR merge, required label missing: dependencies
2023-07-03T09:38:47.228Z INFO  Action result: { mergeResult: 'skipped', pullRequestNumber: 10055 }

If I understand correctly, that means, if if dependencies is added at a later stage of a PR ("PR update"), it is NOT automatically merged. In the udpate case, the label automerge needs to be present.

@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor koppor added the dependencies Pull requests that update a dependency file label Jul 3, 2023
@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

Adding label "dependencies" (and closing and reopening) to trigger automege

@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor koppor marked this pull request as ready for review July 3, 2023 12:56
@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

Note that GitHub's automerge functionality is no option for us. Sometimes, we want to have manual merges. Only in the case of version updates, we want to have automatic ones.

image

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-auto-merge-for-pull-requests-in-your-repository

@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

Blocked by pascalgn/automerge-action#225 (comment)

I merge nevertheless to fix the label type: dependencies (before this PR used by dependabot) and dependencies (before this PR use by gradle update bot)

@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

We want to keep the rule of two reviewers requried. Thus, we cannot apply pascalgn/automerge-action#200 (comment).

Co-authored-by: Houssem Nasri <housi.housi2015@gmail.com>
@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
calixtus
calixtus previously approved these changes Jul 3, 2023
@koppor koppor marked this pull request as draft July 3, 2023 21:34
@koppor
Copy link
Member Author

koppor commented Jul 3, 2023

Last commit tries to make use of pascalgn/automerge-action#61.

@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 3, 2023
@koppor koppor marked this pull request as ready for review July 3, 2023 22:56
@koppor koppor closed this Jul 3, 2023
@koppor koppor reopened this Jul 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@koppor
Copy link
Member Author

koppor commented Jul 4, 2023

@koppor koppor added the status: depends-on-external A bug or issue that depends on an update of an external library label Jul 4, 2023
@koppor koppor marked this pull request as draft July 4, 2023 09:11
@Siedlerchr
Copy link
Member

Closing this because it's not that important and blocked by the other repo

@Siedlerchr Siedlerchr closed this Jul 17, 2023
@koppor koppor added the status: freeze Issues posponed to a (much) later future label Jul 17, 2023
@koppor koppor deleted the fix-auto-approve branch July 17, 2023 21:43
@koppor koppor removed the status: freeze Issues posponed to a (much) later future label Apr 21, 2024
@koppor
Copy link
Member Author

koppor commented Apr 21, 2024

Currently, all auto merge werks. Using GitHub's CLI tool. See https://github.com/JabRef/jabref/blob/main/.github/workflows/automerge.yml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file status: depends-on-external A bug or issue that depends on an update of an external library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants