Skip to content

Development: Use existing builds for protected branches and improve PR lookup to avoid forked PR interference #10719

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

Merged
merged 11 commits into from
Jul 7, 2025

Conversation

egekocabas
Copy link
Member

@egekocabas egekocabas commented Apr 22, 2025

Motivation and Context

Our previous deployment logic had two key issues:

  • Redundant builds for protected branches
    When deploying protected branches like develop or main, which rely on the push event to trigger build.yml, we unnecessarily triggered a new build even when a successful one already existed. This led to wasted CI resources and longer deployment times.
  • Incorrect PR detection due to forked repositories
    When attempting to deploy a branch (e.g., develop), the workflow searched for any open PRs targeting develop. If a user had opened a PR from a fork (e.g., their forked repo’s develop branch), it matched the search query, leading to misaligned deployment behavior.

This PR addresses both issues by:

  • Checking for existing successful builds for protected branches and reusing them when available.
  • Updating PR lookup logic to only consider internal PRs (i.e., those where the source and target repos match), avoiding interference from forked repositories.

Description

  • Added a filter in the PR lookup logic to ensure only internal PRs (non-forked) are considered during deployment checks.
  • Introduced a mechanism to detect and use existing successful builds for protected branches (develop, main), skipping unnecessary rebuilds.

Example Runs

@egekocabas egekocabas temporarily deployed to artemis-test1.artemis.cit.tum.de April 22, 2025 09:53 — with GitHub Actions Inactive
@egekocabas egekocabas temporarily deployed to artemis-test7.artemis.cit.tum.de April 22, 2025 10:15 — with GitHub Actions Inactive
@egekocabas egekocabas changed the title fix: ignore forked prs Development: Ignore forked PRs and skip rebuilds for protected branches during deployment Apr 22, 2025
@egekocabas egekocabas changed the title Development: Ignore forked PRs and skip rebuilds for protected branches during deployment Development: Skip redundant builds for protected branches and ignore forked PRs Apr 22, 2025
@egekocabas egekocabas changed the title Development: Skip redundant builds for protected branches and ignore forked PRs Development: Use existing builds for protected branches and improve PR lookup to avoid forked PR interference Apr 22, 2025
@egekocabas egekocabas temporarily deployed to artemis-test7.artemis.cit.tum.de April 22, 2025 10:35 — with GitHub Actions Inactive
@egekocabas egekocabas marked this pull request as ready for review April 22, 2025 10:46
@egekocabas egekocabas requested a review from a team as a code owner April 22, 2025 10:46
coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 22, 2025
Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@Mtze
Copy link
Member

Mtze commented Apr 30, 2025

@Hialus @bensofficial Can you take a look at this?

@github-actions github-actions bot removed the stale label May 1, 2025
bensofficial
bensofficial previously approved these changes May 12, 2025
Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@github-actions github-actions bot added the stale label May 19, 2025
@egekocabas
Copy link
Member Author

Hello @Hialus, could you also take a look at and also maybe run some deployments to test the functionality?

@egekocabas egekocabas requested a review from Hialus May 19, 2025 15:56
@github-actions github-actions bot removed the stale label May 20, 2025
Copy link

github-actions bot commented Jun 5, 2025

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

bensofficial
bensofficial previously approved these changes Jun 13, 2025
Copy link
Member

@bensofficial bensofficial left a comment

Choose a reason for hiding this comment

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

Reapprove

@egekocabas
Copy link
Member Author

@Hialus could you please take a look at this PR?

Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@egekocabas
Copy link
Member Author

Hey @Hialus do you have a time to check the latest changes?

@github-actions github-actions bot removed the stale label Jun 25, 2025
@egekocabas egekocabas dismissed stale reviews from bensofficial and coderabbitai[bot] via 749be91 July 1, 2025 13:49
Copy link
Member

@Hialus Hialus left a comment

Choose a reason for hiding this comment

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

Changes look sound.

Copy link
Contributor

@ShuaiweiYu ShuaiweiYu left a comment

Choose a reason for hiding this comment

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

The changes look sound.

@krusche krusche added this to the 8.2.3 milestone Jul 7, 2025
@krusche krusche merged commit a7f8e40 into develop Jul 7, 2025
15 of 18 checks passed
@krusche krusche deleted the bugfix/deployment-workflow-pr branch July 7, 2025 17:29
@github-project-automation github-project-automation bot moved this from Ready For Review to Merged in Artemis Development Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

6 participants