paritytech/substrate-telemetry

CI doesn't appear to run for (at least some) frontend PRs

jsdw opened this issue · 5 comments

jsdw commented

No Frontend CI for:

#391
#387

Frontend CI did run for:

#364

Possibly related to some difference in merging from external remotes vs merging from origin?

The same may be true of backend PRs, so it's worth making sure both work as expected when digging into this.

CYBAI commented

I might figure it out why the CI didn't run in my PRs (#391, #387).

on:
push:
paths:
- '.github/workflows/frontend.yml'
- 'frontend/**'
- '!backend/**'

Currently, the CI is configured to be run on push for related paths (frontend for Front End stuff and backend for Back End stuff) which means the CI will only be triggered on push to the upstream repo (paritytech/substrate-telemetry). However, I used a fork repo (cybai/substrate-telemetry) so when I push any changes to a fork repo, the upstream is not pushed and the upstream CI is not triggered.

I wonder maybe adding the same paths configuration for [on.pull_request`](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestpaths) might help 🤔

jsdw commented

Thanks for the pointer; that sounds like the thing to do to me!

I dug into the docs a little more, and this page seems quite relevant:

https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull-request-events-for-forked-repositories

From there, there is also some detail about needing to approve workflow runs from external forks (I suppose so that other users can't maliciously trigger loads of CI workflows in your repository):

https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

My vote would be to just try adding the on.pull_request bits as well as you suggest, and see what happens (I'm sure I've seen that on other repositories we host as well) :)

jsdw commented

I've added the on.pull_request now in the above, so this is ready to test (create a PR from a fork and see if CI runs as expected) and hopefully close.

CYBAI commented

@jsdw Yes! I just rebased #391 to master and it works now! Thank you for the fix!

image

jsdw commented

Awesome, thank you for testing that! I'll close this now :)