Feat: Add Github Action For Unit Tests#59
Conversation
|
This is definitely going in the right direction, but I have a few comments. I think it's better to have many small actions that tell you one specific thing that's wrong with your PR rather than a mega action that could fail for many reasons and you have to go digging in logs every time. For example, https://github.com/bcgit/bc-rust/blob/release/0.1.2alpha/.github/workflows/rust-style.yml Similarly, I would split out |
|
|
||
| pull_request: | ||
| branches: | ||
| - release/0.1.2alpha |
There was a problem hiding this comment.
This probably wants to be release/* so that we don't have to manually bump it for each release.
We could also have a chat about whether we want this to run on all PRs or just the ones targeting a release branch? We're not paying for github server usage (and this action is pretty light anyway), so I don't see a reason to restrict it. That would be:
on:
pull_request:
In this case, I'm not sure that the on: push: trigger is doing anything different from the pull_rquest: trigger? I could be wrong, but I think it would behave the same if you removed that?
@ounsworth ACK. Will split into several smaller Github actions and branches wildcarded into release/* |
|
Can you also make a docsLlint action that runs ? |
|
It would also be cool to put the github CI badges on the main readme. |
29dadd5 to
7120e4a
Compare
…d build workspace (bcgit#45)
8d0dfd5 to
43bc846
Compare
|
@ounsworth rebased off of bcgit release/0.1.3alpha, added in badges into the README.md, and split up CI actions into different yml files Note about rustfmt failing: I verified that the rustfmt failures are in files not modified by this PR and are already present on release/0.1.3alpha. Since cargo fmt --all -- --check checks the entire workspace, the existing formatting debt causes this PR to fail despite not introducing those changes. Repository-wide formatting cleanup should be handled separately, not in this PR. |
Github action to handle (#45) that does the following: