Skip to content

Conversation

@tuhaihe
Copy link
Member

@tuhaihe tuhaihe commented Jun 19, 2025

For the contexts, should be the names of checks that must pass, which are the jobs' name property, instead of the steps' id value.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@tuhaihe tuhaihe changed the title CI: update the must passed jobs' name in .asf.yaml [skip ci ] CI: update the must passed jobs' name in .asf.yaml Jun 19, 2025
@tuhaihe tuhaihe changed the title [skip ci ] CI: update the must passed jobs' name in .asf.yaml [skip ci] CI: update the must passed jobs' name in .asf.yaml Jun 19, 2025
@tuhaihe tuhaihe marked this pull request as draft June 19, 2025 08:55
@tuhaihe tuhaihe force-pushed the update-check-context branch from 7ee82a0 to 37efa4f Compare June 19, 2025 09:35
@tuhaihe tuhaihe changed the title [skip ci] CI: update the must passed jobs' name in .asf.yaml CI: update the must passed jobs' name in .asf.yaml Jun 19, 2025
@tuhaihe tuhaihe marked this pull request as ready for review June 19, 2025 10:03
@tuhaihe tuhaihe requested a review from edespino June 19, 2025 10:04
@tuhaihe
Copy link
Member Author

tuhaihe commented Jun 20, 2025

Can see the checks are set as required, except:

  • Apache Cloudberry Build / prepare-test-matrix (pull_request)
  • Apache Cloudberry Build Debug / check-skip (pull_request)
  • Apache Cloudberry Build Debug / ic-cbdb-parallel (pull_request)
  • Apache Cloudberry Build Debug / prepare-test-matrix (pull_request)

The results are the same as the original set, like the checks in #1171

Copy link
Contributor

@leborchuk leborchuk left a comment

Choose a reason for hiding this comment

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

Looks good for me, job-id we use

``
xifos@localhost$ grep "Job:" .github/workflows/build-cloudberry.yml

Job: check-skip

Job: prepare-test-matrix

Job: build

Job: rpm-install-test

Job: test

Job: report

``
I also checked - the actual IDs are the same as in description

For the contexts, should be the names of checks that must pass, which
are the jobs' `name` property, instead of the steps' id value.
@tuhaihe tuhaihe force-pushed the update-check-context branch from 24d5131 to de6c197 Compare October 23, 2025 09:40
Copy link
Contributor

@edespino edespino left a comment

Choose a reason for hiding this comment

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

LGTM!

This PR correctly updates the required status checks in .asf.yaml to use job IDs instead of job display names.

Key Changes Verified:

  • ✅ Changed from display names (Build Apache Cloudberry RPM, RPM Install Test Apache Cloudberry, etc.) to job IDs (build, rpm-install-test, etc.)
  • ✅ Simplified matrix job checks - using parent job ID test instead of listing all matrix combinations individually
  • ✅ All job IDs match the actual job definitions in .github/workflows/build-cloudberry.yml:
    • check-skip (line 164)
    • prepare-test-matrix (line 207)
    • build (line 413)
    • rpm-install-test (line 693)
    • test (line 876)
    • report (line 1742)

Testing Notes:

Validated the workflow trigger behavior and confirmed that GitHub's branch protection matches status checks by job ID (the YAML key under jobs:), not by the optional name: field. This change ensures branch protection will correctly wait for the required checks before allowing merge.

The simplified approach is more maintainable and aligns with GitHub Actions best practices.

Technical Background:

GitHub matches required status checks using job IDs from workflow files, not the display names. When multiple workflows have jobs with the same ID (e.g., both build-cloudberry.yml and build-dbg-cloudberry.yml have a build job), GitHub will wait for all matching jobs to complete. This PR correctly identifies the job IDs that should be required for PRs to main.

@tuhaihe tuhaihe merged commit 494a560 into apache:main Oct 23, 2025
28 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.

3 participants