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

Fix CI status checks on PR from forks #622

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

istandre
Copy link
Contributor

@istandre istandre commented Jan 20, 2024

Resolve #621

Description:

Pull requests coming from forks do not trigger the CI workflows. This is because the workflows only wait for a push and this cannot happen in this specific context.

This commit fixes the issue by also triggering workflows on pull_request events.

You can see that the workflows are waiting approval from a maintainer before starting: backend (3.9) action workflow

Checklist:

You should check all boxes before the PR is ready. If a box does not apply, check it to acknowledge it.

  • ISSUE NUMBER. You linked the issue number (Ex: Resolve #XXX).
  • PRE-COMMIT. You ran pre-commit on all commits, or else, you
    ran pre-commit run --all-files at the end.
  • USER CHANGES. The changes are added to CHANGELOG.md and the documentation, if they impact
    our users.
  • DEV CHANGES.
    • Update the documentation if this PR changes how to develop/launch on the app.
    • Update the README files and our wiki for any big design decisions, if relevant.
    • Add unit tests, docstrings, typing and comments for complex sections.

@istandre
Copy link
Contributor Author

Note that if you would like to trigger the workflows regardless of approval status, the pull_request_target can also be added to the list. This will start the workflows, but in the base branch context. This to avoid abusive purposes, since the base branch is considered trusted.

Copy link
Contributor

@JosephMarinier JosephMarinier left a comment

Choose a reason for hiding this comment

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

Hello @istandre,

Thank you so much for looking into this! I have had a crazy week and I didn't find the time to look into #620 seriously. I am relieved that you found the issue. 😌

Thanks also for the very clear explanations. I didn't know any of this. 👍

Given the low volume of PRs on this repo, I think we'll be OK with approving the CI runs for now. I'll keep the pull_request_target option for the future.

Thanks again!

@JosephMarinier JosephMarinier enabled auto-merge (squash) January 20, 2024 04:52
@JosephMarinier JosephMarinier merged commit 21ea4c4 into ServiceNow:main Jan 20, 2024
2 checks passed
@istandre istandre deleted the status_checks branch January 20, 2024 05:07
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.

Status checks do not run for PR submitted from forks
2 participants