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

Ci workflow #5

Merged
merged 3 commits into from
Jul 12, 2023
Merged

Ci workflow #5

merged 3 commits into from
Jul 12, 2023

Conversation

george-cosma
Copy link
Contributor

@george-cosma george-cosma commented May 6, 2023

This PR contains the implementation of a basic CI workflow for rust.
In short, when someone pushes or makes a pull request, the following tests are performed:

  • cargo build succeeds
  • All test pass (cargo test)
  • No formatting issues with rust-fmt
  • No issues with clippy

The Makefile has also been updated so that the ci-runner-github matches the same tests that are done by the CI Workflow (it wasn't running cargo test previously).

Also, here is a sample pull requests that fails: george-cosma#1

@alexandruradovici
Copy link
Collaborator

As this uses git in the action commands, this might not work with merge queue. I suggest we merge this and try.

@george-cosma
Copy link
Contributor Author

For the merge queue to work, it might need the merge_group1 trigger added, but even then I'm not sure it will work - I can't enable the merge queue on my fork to test. Should I add it, and see if it works?

Footnotes

  1. https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue#triggering-merge-group-checks-with-github-actions

hudson-ayers
hudson-ayers previously approved these changes May 10, 2023
Copy link

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I figure just give it a shot!

@george-cosma
Copy link
Contributor Author

This change should enable the CI tests to run on the merge queue if the repository is configured correctly.

A note on settings

For the merge queue to function as expected, it seems that both the "Require merge queue" and "Require status checks to pass before merging" need to both be active at the the same time (Thanks to @alexandruradovici for discovering this). If the second rule is not present, the merge queue will not stop a failing PR from being merged.

I've documented my test attempts over here UPB-CS-OpenSourceUpstream#14

@alexandruradovici
Copy link
Collaborator

Did it run all the tests while in the merge queue?

@george-cosma
Copy link
Contributor Author

Yes, it ran all of them.
Here's the link to the test it ran https://github.com/UPB-CS-OpenSourceUpstream/tockloader-rs/actions/runs/4958024994 for PR UPB-CS-OpenSourceUpstream#15

@alexandruradovici
Copy link
Collaborator

As soon as @hudson-ayers approves it, I will merge this manually and enable the merge queue.

Copy link

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this slipped through the cracks, but looks good!

@ppannuto ppannuto merged commit 99d029f into tock:main Jul 12, 2023
@ppannuto
Copy link
Member

Merged & merge queue enabled on main branch

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.

None yet

4 participants