Skip to content

feat(ci): always test before publish #843

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.

Sign up for GitHub

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

Jump to bottom
Merged
lorenjohnson merged 1 commit into from
Mar 25, 2025
Merged

feat(ci): always test before publish #843

lorenjohnson merged 1 commit into from
Mar 25, 2025

Conversation

@lorenjohnson
Copy link
Contributor

@lorenjohnson lorenjohnson commented Mar 21, 2025 •
edited
Loading

Skips re-run of CI runs when there is already a CI run completed. This is a continuation of work/discussion in https://phabricator.wikimedia.org/T378941 and #801:

There are several "don't merge" PRs which serve as baseline proofs of the skip logic here:

843-test-1: Skips CI run on docs only change
843-test-2: Runs CI on code change
843-test-3: Skip CI on no new code change
843-test-4: Runs CI on failing code change
843-test-5: Runs CI again on duplicate failing code change

NOTE: The _gha_test_main branch has the "success" branch protection rule that we have on main, but points to the new "success" that is now located in the build_and_test workflow vs in _build_and_test. Before this branch would be merged we would want to change the rule for main to point to this new "success" check as it is setup for the _gha_test_main branch

@lorenjohnson lorenjohnson changed the base branch from main to test-before-release March 21, 2025 10:47
@lorenjohnson lorenjohnson changed the base branch from test-before-release to main March 21, 2025 10:48
@lorenjohnson lorenjohnson force-pushed the _gha_test_main branch 2 times, most recently from 3cfa1c8 to 73c961f Compare March 21, 2025 13:10
@lorenjohnson lorenjohnson reopened this Mar 21, 2025
@lorenjohnson lorenjohnson requested a review from a team March 21, 2025 23:40
@lorenjohnson lorenjohnson marked this pull request as draft March 21, 2025 23:44
@lorenjohnson lorenjohnson removed the request for review from a team March 21, 2025 23:44
@lorenjohnson lorenjohnson force-pushed the _gha_test_main branch 2 times, most recently from dd42912 to 39512ce Compare March 24, 2025 10:29
@lorenjohnson lorenjohnson changed the title feat(ci): always test before publish (continuation) feat(ci): always test before publish Mar 24, 2025
@lorenjohnson lorenjohnson requested a review from a team March 24, 2025 15:01
@lorenjohnson lorenjohnson marked this pull request as ready for review March 24, 2025 15:01
@lorenjohnson
Copy link
Contributor Author

lorenjohnson commented Mar 24, 2025 •
edited
Loading

NOTE: The CI run on this branch will not pass _success as it is targeted to main with branch protection rules which look to the old _build_and_test workflow _success job, while otherwise running the new build_and_test from this branch on this branch... If that makes sense. The test branches that succeed will be what things will look like on a normal PR once this is integrated.

rti
rti reviewed Mar 25, 2025
View reviewed changes
Copy link
Contributor

@rti rti left a comment

Choose a reason for hiding this comment

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

I documented my thought process here a bit. "Why are there multiple workflow runs?" But I think I now get it. As the PRs are always referencing the exact same commit, they trigger multiple workflow runs on the same commit. But the skip logic prevents the full execution of the run.

The skip-duplicate-actions docs state:

If there exists a workflow run with the same tree hash, then we have identified a duplicate workflow run.

So I was curious to see if it also works on new commits that do not change the tree hash. This is why I created this PR. The skip worked here too.

Sorry for the noise if there were no news in here for you. :)

@lorenjohnson lorenjohnson requested a review from rti March 25, 2025 10:06
@lorenjohnson
feat(ci): Ensures merges to main branch and publish events have passi…
7d55b73 
…ng CI, eliminates duplicate CI runs for PRs (T378941)
@lorenjohnson lorenjohnson merged commit cf5b4bb into main Mar 25, 2025
7 checks passed
@lorenjohnson lorenjohnson deleted the _gha_test_main branch March 25, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rti rti rti approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lorenjohnson @rti