-
Notifications
You must be signed in to change notification settings - Fork 147
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
chore(ci): run workflows based on file changes #1147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we avoid the dispatch and have each workflow perform the file checks ?
Basically a first job run on github ubuntu instance generates the changed files that are relevant for the current workflow, if there are changes then continue in the workflow with the instance set up ?
I don't know if the indirection created by the workflow call is a good idea, is that the pattern concrete used ?
Concrete use the dispatch pattern indeed. Here's the pros and cons of this approach:
Regarding your proposal (one file check in each workflow). I think it's a better approach in our case. We define once and for all the file-change check and we carry only one output in the job. Thus we eliminate the need to define long I'll give a shot to your proposal then. |
d4030ae
to
33043c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is just a tiny bit more work to do to manage file dependency chains, that could be detected in the file chnange step
basically we change core, then we want to re run everything that depends on it
csprng likely means core needs to re run etc.
33043c7
to
75d7e79
Compare
75d7e79
to
09ef9b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
looks like we started instances but then ran no tests https://github.com/zama-ai/tfhe-rs/actions/runs/9254705023/job/25457030476?pr=1147 ? |
probably a check missing to not start instances at all if no file changed |
You're right. Have to fix this. |
This is done to imporve iteration time and feedback for devs. There is no point to run the full test suite each time. A given development could impact only tfhe-zk-pok for example. In this case only tfhe-zk-pok test would run and thus cutting workflow duration from around 3 hours down to a few minutes.
09ef9b6
to
560c589
Compare
This is done to imporve iteration time and feedback for devs.
There is no point to run the full test suite each time. A given development could impact only tfhe-zk-pok for example. In this case only tfhe-zk-pok test would run and thus cutting workflow duration from around 3 hours down to a few minutes.