-
Notifications
You must be signed in to change notification settings - Fork 4
Fix CI check for code changed (suggested by Zach) #1410
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
Conversation
With the additional code added to check if this is the first commit on a branch, I think this looks good! |
That great - on my branch the check evaluates to true when i pushed a trivial code change. But for some reason the tests are still being skipped. I feel like i must be missing something really stupid. https://github.com/entropyxyz/entropy-core/actions/runs/14771602673/job/41472444384?pr=1392 |
The only thing I can think of, at a glance, is that the steps feel a little roundabout. The code checkout echoes to |
Ah, looking at it closer, I think I see the issue with the other job. My commit above will fix it, indirectly, and so I still want to explain it here. On line 10 of the workflow file, you set the output as: outputs:
run_tests: ${{ steps.filter.outputs.changes_detected }} And the steps are then: - name: Check for relevant changes
id: filter
run: |
## echo values to $GITHUB_ENV
- name: Set output
run: # echo env values to $GITHUB_OUTPUT The outputs definition uses the outputs of the step with the So another solution would be to set the |
Thats great - using this correctly triggered the tests to run. Yeah i see now that that code was pretty stupid. But whats strange is that it's been like that for several weeks and no-one including myself has complained about it until now. Thanks so much @cooldracula |
In some cases tests were not being run despite code having been changed, due to an issue with the check as to whether code changes. Eg: https://github.com/entropyxyz/entropy-core/actions/runs/14751508488/job/41409844605?pr=1392
@cooldracula suggested this fix - making it so that all commit history is checked out, so
git diff
can find the commits we are referring to.