-
Notifications
You must be signed in to change notification settings - Fork 553
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 GitHub actions version of the CI #2109
Conversation
12a7440
to
9cfc077
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 @nbdd0121, this LGTM
I checked that the action matches ibex-rtl-ci-steps.yml
and that the workflow matches azure-pipelines.yml
. The scripts all look the same, except obviously with the Azure-specific variables ported as well as the error message format converted.
Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
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.
Looks good thanks for doing this @nbdd0121
We don't need to do this for this PR but I think it'd be reasonable to move to Ubuntu 22 for the Ibex github actions. We'll need to build a verilator binary for it but that's straightforward. Everything else should just work.
We should also look at moving to a newer verilator version (again will require building new binaries)
Upgrading Ubuntu and verilator version sounds great. I'll add that to my TODO list. |
Discussed with @nbdd0121, I would suggest disabling the Azure pipeline in the web UI and leaving the configuration in this repo for at least one real run so that we can re-enable it if there's an issue. |
I have disabled the pipeline. I notice that the GitHub Actions pipeline didn't run during the merge queue, do we need to add it to some list for that, or will it run on the next merge @nbdd0121? Also, if it runs in the merge queue, should we disable the |
No description provided.