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

Why is the action limited to pull_request: [opened, reopened]? #648

Closed
rajyan opened this issue Feb 9, 2024 · 4 comments · Fixed by #654
Closed

Why is the action limited to pull_request: [opened, reopened]? #648

rajyan opened this issue Feb 9, 2024 · 4 comments · Fixed by #654

Comments

@rajyan
Copy link
Contributor

rajyan commented Feb 9, 2024

Currently the Codium-ai/pr-agent Github Action runs pr-agent on pull_request: [opened, reopened]

https://github.com/Codium-ai/pr-agent/blob/e4f177908b620e46740b03966fda9243473d979e/pr_agent/servers/github_action_runner.py#L82

I think it is a sensible default (running pr-agent in 'synchronize' would get flooded), but there are lot of use cases that are reasonable to run pr-agent in 'synchronize' or 'ready_for_review'.

For example

  • Run when 'ready_for_review' for draft pull requests instead of opened, because in most cases /review, /describe or /improve for draft pull requests are useless
  • Run pr-agent on 'synchronize' when labeled with a specific label

Simply put, we want to have control over when the PR agent runs according to our own conditions.

Is it possible to change this line into a config? or are there any reasons that the event is restricted to [opened, reopened]?

@mrT23
Copy link
Collaborator

mrT23 commented Feb 11, 2024

Hi @rajyan
Github action is a limited feature.

If you want full control, use PR-Agent Pro 💎 app, where you can modify everything (for example, running on push events: https://github.com/Codium-ai/pr-agent/blob/main/Usage.md#github-app-automatic-tools-for-push-actions-commits-to-an-open-pr )

I won't enable running pr-agent GitHub action on 'synchronize' for two reasons:
-too intrusive and wasteful for most people
-every time you run GitHub action, you pull a docker. This operation costs us money, and with large numbers, it's not a negligible amount. 'synchronize' will force us to use far more expensive docker hosting solutions.

You can open a PR for adding 'ready_for_review'. It is reasonable.
But PR-Agent Pro 💎 will give you the full control you want, as well as many other advantages.

@rajyan
Copy link
Contributor Author

rajyan commented Feb 11, 2024

@mrT23

Thank you for your explanation.

-too intrusive and wasteful for most people

I totally agree with your point.

https://github.com/Codium-ai/pr-agent/blob/e4f177908b620e46740b03966fda9243473d979e/pr_agent/servers/github_action_runner.py#L82
I was wondering if this line line could be configured as a config (with default as the current ["opened", "reopened"])

-every time you run GitHub action, you pull a docker. This operation costs us money

I couldn't get your point for this part. Running GitHub Actions builds from Dockerfile.github_action_dockerhub as configured here https://github.com/Codium-ai/pr-agent/blob/79dac3f419f11c6c9e7dcc2410418b6d0473b269/action.yaml (int the running environment), so I believe it wouldn't affect your cost.

You can open a PR for adding 'ready_for_review'. It is reasonable.

I'll open it soon!

But PR-Agent Pro 💎 will give you the full control you want, as well as many other advantages.

Thanks for your advice!
We are interested in GitHub App too, but can it do same things that GitHub Actions can do?

If we use GitHub Actions, we can combine the pr-agent action with other actions. For example, we can run https://github.com/dorny/paths-filter in a previous step, and only run the review if specific set of files were changed in the pull request.

@mrT23
Copy link
Collaborator

mrT23 commented Feb 11, 2024

Every time you run pr-agent as a GitHub action, you pull our docker from dockerhub.
This creates a lot of traffic for our dockerhub account.
'synchronize' can increase the traffic by ~x10 - hence no.

image

@rajyan
Copy link
Contributor Author

rajyan commented Feb 11, 2024

@mrT23

Thanks you for your explanation!

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 a pull request may close this issue.

2 participants