Skip to content

Conversation

@keyvaann
Copy link
Collaborator

Only run the full tests when the CI jobs are running on the main branch or a PR to the main branch

@keyvaann keyvaann requested a review from pvannierop July 30, 2024 09:33
@keyvaann keyvaann force-pushed the only-test-on-main branch from bdf1a72 to 490790b Compare July 30, 2024 09:43
Copy link
Contributor

@pvannierop pvannierop left a comment

Choose a reason for hiding this comment

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

Very elegant! I think that this section:

pull_request:
    branches:
      - main
      - dev

will cause the tests only to run when there is a PR from a branch that is named main or dev. What we want is to run tests on any PR that targets main or dev. Would this work:

pull_request_target:
    branches:
      - main
      - dev

See: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onpull_requestpull_request_targetbranchesbranches-ignore

@keyvaann
Copy link
Collaborator Author

I think the difference between pull_request and pull_request_target is more about security rather than functionality.
https://stackoverflow.com/questions/74957218/what-is-the-difference-between-pull-request-and-pull-request-target-event-in-git

@pvannierop
Copy link
Contributor

But my concern remains:

will cause the tests only to run when there is a PR from a branch that is named main or dev.

@keyvaann keyvaann force-pushed the only-test-on-main branch 4 times, most recently from 119e8d6 to 68678fa Compare July 31, 2024 10:37
@keyvaann keyvaann force-pushed the only-test-on-main branch from a7f03da to f0ed2ce Compare July 31, 2024 13:39
@keyvaann keyvaann force-pushed the only-test-on-main branch from 33eaed0 to 0a94921 Compare July 31, 2024 15:47
@keyvaann keyvaann merged commit 84ff154 into dev Aug 5, 2024
@keyvaann keyvaann deleted the only-test-on-main branch August 5, 2024 07:44
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