-
Notifications
You must be signed in to change notification settings - Fork 119
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
Add enzyme/tools to clang-format check #1221
base: main
Are you sure you want to change the base?
Conversation
@tgymnich @wsmoses iirc we had a reason to exclude PRs? |
Refer to https://stackoverflow.com/questions/58457140/dependencies-between-workflows-on-github-actions , it seems we can add |
@wsmoses any objections to it? |
@ZuseZ4 yeah if its already run on every commit push, also running for PR (which would require a commit) is redundant. |
I'm fine with the don't run the rest of CI, but let's enable it for everything except the LLVM CI (that way it can still check that it builds on all versions), which takes the least time. |
Sounds good |
Thanks, I can get your point. But I think it is not friendly to outside contributors who fork this repo and open pull requests from their own fork, in which case these CI status does not appear in the PR page. It seems an alternative solution is something like:
So that if you push to a branch |
That sounds good, feel free to implement it this way :) |
LGTM. There are just a few minor formatting issues left. |
Conflict has been solved now. |
It seems this method cannot work well, i.e. these workflows by Maybe need some investigation or workaround, or just keep the original behavior and try it in another PR. |
Maybe the root cause is from the quote. Some trying... |
It does look like Integration CI is now blocked on all Enzyme CI tests passing. Especially the apple runners take a while, so by now it would increase our CI times by a good amount. Can you update it please to make sure that we are only waiting for the clang-format check to pass before starting Integration CI? Edit: looks like we noticed it simultaneously. |
🤣
Some try, still not triggered. I will revert all these |
* Runtime splat * fix internal side
Some misc work are done in this PR:
.clang-format
to the top directory, as a common waypull_request
event for the clang-format check CIClose #1211. cc @wsmoses @ZuseZ4