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

[CI] Fix workflow to check use of private emails #17333

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

uditagarwal97
Copy link
Contributor

@uditagarwal97 uditagarwal97 commented Mar 6, 2025

https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull_request

By default, a workflow only runs when a pull_request event's activity type is opened, synchronize, or reopened.

By adding types: - opened, we are restricting this workflow to run only when the PR is first opened. I don't see a good reason to do that. This PR removed it and also makes sure that this workflow is triggered only on certain branches.

@uditagarwal97
Copy link
Contributor Author

I think the reason why this workflow didn't get triggered on #17065 is because we were missing the synchronize event type in this workflow. So, using the defaults pull_request trigger type (open, re-open, sync) should fix it.
https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=synchronize#pull_request

@uditagarwal97
Copy link
Contributor Author

But, here's a downside to this change: The workflow will issue multiple comments (if the user is not using intel/codeplay email), every time a pull-request is re-opened or synchronized. I personally think it's not that big of an issue as it won't affect most contributors of intel/llvm

@uditagarwal97 uditagarwal97 marked this pull request as ready for review March 6, 2025 17:14
@uditagarwal97 uditagarwal97 requested a review from a team as a code owner March 6, 2025 17:14
@uditagarwal97 uditagarwal97 requested a review from sarnex March 6, 2025 17:14
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

change seems correct me, this should run on every commit in a PR

@uditagarwal97
Copy link
Contributor Author

Canceling pre-commit CI and merging - this change shouldn't affect SYCL Build or testing.

@uditagarwal97 uditagarwal97 merged commit 416eb46 into sycl Mar 6, 2025
9 of 14 checks passed
@bader bader deleted the uditagarwal97-patch-2 branch March 11, 2025 19:36
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.

2 participants