Skip to content
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

Merged
merged 2 commits into from
Dec 15, 2023
Merged

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Dec 7, 2023

No description provided.

@nbdd0121 nbdd0121 force-pushed the master branch 4 times, most recently from 12a7440 to 9cfc077 Compare December 7, 2023 19:23
@nbdd0121 nbdd0121 requested a review from jwnrt December 8, 2023 14:02
Copy link
Contributor

@jwnrt jwnrt left a 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.

.github/workflows/ci.yml Show resolved Hide resolved
Copy link
Collaborator

@GregAC GregAC left a 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)

@nbdd0121
Copy link
Contributor Author

Upgrading Ubuntu and verilator version sounds great. I'll add that to my TODO list.

@nbdd0121 nbdd0121 added this pull request to the merge queue Dec 15, 2023
@jwnrt
Copy link
Contributor

jwnrt commented Dec 15, 2023

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.

Merged via the queue into lowRISC:master with commit 1c5da19 Dec 15, 2023
12 checks passed
@jwnrt
Copy link
Contributor

jwnrt commented Dec 15, 2023

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 on: push: branches: ["*"] run which I think will be redundant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants