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

feat: pull requests are automatically merged based on their activity #1

Merged
merged 107 commits into from
Jul 29, 2024

Conversation

gentlementlegen
Copy link
Member

@gentlementlegen gentlementlegen commented Jul 2, 2024

@gentlementlegen gentlementlegen marked this pull request as draft July 3, 2024 14:31
@gentlementlegen
Copy link
Member Author

@rndquu I changed the token to be the one from the repo itself as @whilefoo seemed to suggest in this comment if I understood properly, please let me know if that fixed your issue.

@gentlementlegen gentlementlegen requested a review from 0x4007 July 17, 2024 07:16
@rndquu
Copy link
Member

rndquu commented Jul 17, 2024

@rndquu I changed the token to be the one from the repo itself as @whilefoo seemed to suggest in this comment if I understood properly, please let me know if that fixed your issue.

The error is gone but the PR is not merged.

Check this config and this CI run which prints this log output:

issue_comment.created is not supported, skipping

Why issue_comment.created is not supported? Expected behavior: on this comment this PR is merged.

@whilefoo
Copy link
Member

Check this config and this CI run which prints this log output:

issue_comment.created is not supported, skipping

Why issue_comment.created is not supported? Expected behavior: on this comment this PR is merged.

Even though that may seem like an error it isn't, the code should be modified to not look like it failed.
I think the PR is not merged because there are no approvals

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Jul 18, 2024

@rndquu I changed the token to be the one from the repo itself as @whilefoo seemed to suggest in this comment if I understood properly, please let me know if that fixed your issue.

The error is gone but the PR is not merged.

Check this config and this CI run which prints this log output:

issue_comment.created is not supported, skipping

Why issue_comment.created is not supported? Expected behavior: on this comment this PR is merged.

The comment means there will be no action taken because this event does not trigger any logic inside the plugin. I can change the message, not sure what could make it clearer?

Also the PR didn't close for 2 reasons: one is that I think you opened it before the plugin was properly working on the repo so it is actually not tracked. Two is that there is no reviewer that approved so would be skipped.

My latest test run: Meniole#6

@gentlementlegen gentlementlegen requested a review from whilefoo July 18, 2024 23:55
@whilefoo
Copy link
Member

The comment means there will be no action taken because this event does not trigger any logic inside the plugin. I can change the message, not sure what could make it clearer?

It looks like the plugin didn't even run but in fact it did run updatePullRequests, just the proxy callback didn't match the event

@gentlementlegen
Copy link
Member Author

@whilefoo Correct, that is the intended behavior. I can try to make a clearer message if that is necessary.

@rndquu
Copy link
Member

rndquu commented Jul 25, 2024

@gentlementlegen Check this config, this PR and this CI run.

On any comment I'm expecting this PR to be merged but this CI run didn't merge it. What am I doing wrong?

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Jul 25, 2024

@rndquu I think the problem is that it only runs on comment_created but it should also run on pull_request.opened. So the PR is never added to the database hence the nothing to do in the logs.
https://github.com/ubiquibot/automated-merging/blob/22c79f0126b1cab92b91d07b59054db57f61b927/src/proxy/index.ts#L11

@rndquu
Copy link
Member

rndquu commented Jul 25, 2024

@rndquu I think the problem is that it only runs on comment_created but it should also run on pull_request.opened. So the PR is never added to the database hence the nothing to do in the logs.

https://github.com/ubiquibot/automated-merging/blob/22c79f0126b1cab92b91d07b59054db57f61b927/src/proxy/index.ts#L11

Could you provide a valid config for QA?

@gentlementlegen
Copy link
Member Author

@rndquu My config is like this:

  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

I am using the version of the Kernel that retrieves the data from the manifest.json.

@rndquu
Copy link
Member

rndquu commented Jul 25, 2024

@rndquu My config is like this:

  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

I am using the version of the Kernel that retrieves the data from the manifest.json.

How should the whole config look like with pull_request.opened for the plugin to work as expected?

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Jul 25, 2024

@rndquu It can just be like

plugins:
  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

and the automated-merging repo should contain a manifest like

{
  "name": "Automated merging",
  "description": "Automatically merge pull-requests.",
  "ubiquity:listeners": [ "push", "pull_request.opened", "pull_request.reopened" ]
}

@rndquu
Copy link
Member

rndquu commented Jul 26, 2024

@rndquu It can just be like

plugins:
  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

and the user-activity-watcher repo should contain a manifest like

{
  "name": "Automated merging",
  "description": "Automatically merge pull-requests.",
  "ubiquity:listeners": [ "push", "pull_request.opened", "pull_request.reopened" ]
}

How user-activity-watcher is connected with the automated-merging plugin?

@gentlementlegen
Copy link
Member Author

@rndquu my mistake, got confused while writing it, fixed my comment, meant to say automated-merging.

src/action.ts Show resolved Hide resolved
src/adapters/index.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@0x4007
Copy link
Member

0x4007 commented Jul 28, 2024

As long as you tested it works I think just merge and we can test it out in production.

Co-authored-by: アレクサンダー.eth <4975670+0x4007@users.noreply.github.com>
@gentlementlegen
Copy link
Member Author

@rndquu I'll merge this because it is needed for the v2, but feel free to test and let me know if there is anything wrong, I'll fix within the following pull-request.

@gentlementlegen gentlementlegen merged commit 8da60bb into ubiquity-os-marketplace:development Jul 29, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic Merge In Timeout
4 participants