search

riker-rs / riker

Easily build efficient, highly concurrent and resilient applications. An Actor Framework for Rust.
https://riker.rs
MIT License
1.01k stars 71 forks source link

Is there any interest in having GH Actions run clippy, rustfmt and tests on PRs? #142

Closed nothingismagick closed 3 years ago

nothingismagick commented 3 years ago

I can make a PR that will introduce these (at least on Linux, but can also run a matrix for MacOS and Windows)... For public projects like this one there is no fee for running these workflows...

hardliner66 commented 3 years ago

I think currently we use travis to run these commands. I personally think that Github actions would be better, because we wouldn't need another service.

Other than that, I can't say which one is better because I haven't worked with either of them. Maybe someone else can comment on that topic.

nothingismagick commented 3 years ago

Yeah, I haven't used travis in years, but don't know where to look to see if the run passed. I can also just clone that travis workflow into a GH workflow...

nothingismagick commented 3 years ago

Hmm. npm being flaky?

https://travis-ci.org/github/riker-rs/riker/jobs/745630770

nothingismagick commented 3 years ago

I just ran pre-commit run -a against current master and everything passed...

hardliner66 commented 3 years ago

Could be. We had some CI problems before. Maybe @olexiyb can check what went wrong. He worked on the CI stuff.

I think we should work towards a more stable CI. And if Github Actions helps, then i'm all for it. Could you make a test branch in your fork so that we can have a look at it?

nothingismagick commented 3 years ago

Sure, will do.

nothingismagick commented 3 years ago

Here is a PR to my forked master branch that uses cargo audit, clippy, rustfmt (nightly), builds and tests on macOS, Linux, Windows. I also updated the badge on the readme to point at the test results. Will modify this to point at the riker org if you accept this and want a PR.

Note here, that the clippy warnings are exceptionally useful and highly readable: https://github.com/nothingismagick/riker/pull/3/checks?check_run_id=1452948120

nothingismagick commented 3 years ago

Also if you scroll down to the end, here: https://github.com/nothingismagick/riker/pull/3/files

You will see the clippy comments. If they are found in the current PR's files, they will be inlined there.

nothingismagick commented 3 years ago

Another nice thing with GH Actions is that you can protect a branch and prevent even administrators from merging if any workflows fail... I love that feature.

nothingismagick commented 3 years ago

We could also run all the examples to make sure that they still work too...

olexiyb commented 3 years ago

I have added pre-commit and changed existing Travis to run pre-commit checks those include clippy, check, fmt and prettier (to enforce default spacing styles) I would highly recommend keeping pre-commit support as it allows to enforce to run all checks during commit unless Github has a way to run these checks from the command line without PR.

olexiyb commented 3 years ago

I think you use the latest rust version and Clippy, that's why you may see new errors.

nothingismagick commented 3 years ago

Sure - I didn't add precommit because I just wanted to show the approach, but don't think it would be a big problem to add if there is consensus that this project wants to start using GH Actions.

olexiyb commented 3 years ago

I have created a pull request https://github.com/nothingismagick/riker/pull/4 to use pre-commit GH action.

leenozara commented 3 years ago

I'm currently reviewing GH Actions. Precommit has been an issue on and off since we introduced it, but as @olexiyb mentions it serves as an important step to ensure the integrity of code upfront. Part of the problem has been the integration between GH and Travis changing over time, such as webhooks.

@nothingismagick I'll look at your fork for GH Actions, thanks.