After very recently syncing with Troy, the owner of the Rune project (you can check my first contribution blog post here), we were discussing about the next (immediate) steps for the project, and overall my next contribution. Here's my proposal:
Step by step, we need CI for pull requests and then we can potentially work on simplifying and completing the backlog of issues we have. With a prioritized backlog, we can call for contributions on both Emacs and Rust’s subreddits. What do you think?
After the CI, I can build something with async rust (probably file io can benefit from it), and make sure to measure it with benchmarks (criterion or cargo benchmark). Those benchmarks could potentially be added to CI once we are happy with them, and make sure we don’t regress performance with pull requests or commits.
Looking forward to next steps, Best, Quique.
With that, here's the first step: CI for the Rune project, buckle up!
I was aware that Jon Gjenset (@jonhoo) did a super nice video explaining his recommendation for CI that should run on lib/bin crates (ref). That video goes through an overview of some files of the project rust-ci-conf, which is a set of Github Actions workflows to cover the bare minimum for crates: stepping stones to a great CI pipeline for our Rune project.
With that, I used the project as a blueprint of what CI could look like and started modifying the workflows with the specific requirements of the rune crate. From the top of my head:
beta and that broke CI and is not common in other popular crates.cargo-hack wouldn't do us any good.unsafe code, which means we need to care about safety and ensure any change is sanitized and avoids any Undefined Behaviour.Staring from those requirements, I modified the DOCS.md file of the .github folder with the intention of documenting possible improvement points I see on CI, with the intention of future contributors iterating on the current CI pipeline.
cargo-hackchecks combinations of feature flags to ensure that features are all additive which is required for feature unification.
As we don't currently have any meaningful feature gates.
Run scheduled (rolling) jobs on a nightly basis, as your crate may break independently of any given PR. E.g., updates to rust nightly and updates to this crates dependencies. See check.yml for information about how the concurrency cancelation and workflow triggering works
As we don't run on nightly, we don't really need to check whether nightly breaks our builds. A potential argument for adding this could be that we are running nightly for the CI, and we could break the CI from nightly builds. I propose revisiting adding the scheduled.yml linked above if that's the case.
Loom is a testing tool for concurrent Rust code. It runs a test many times, permuting the possible concurrent executions of that test under the C11 memory model. It uses state reduction techniques to avoid combinatorial explosion.
Loom is great and it's backed by Tokio, but it would mean a bigger investment in making the threads we use be loom specific. Definitely something to iterate on, as we get onto Async Emacs.
Enhance Your Testing the Codecov Way: Codecov is the all-in-one code coverage reporting solution for any test suite — giving developers actionable insights to deploy reliable code with confidence.
If we want to add a badge or have nice reports on coverage, we could investigate CodeCov. It requires a token that should be added to the secrets of the repository: CODECOV_TOKEN.
matrix:
os: [macos-latest] # REVIEW: We could add windows-latest here.
As of today, we don't support Windows, or at least the tests are failing. We can iterate on this and if we feel Windows should be supported in the near feature, we can add windows-latest to run CI on it.
As suggested by issue, doctests won't currently run on Binary creates. I don't see anything particularly wrong with running doctests on binary crates. Some of the argumenst include that "Why would a binary be documented?". Documentation is not only useful for consumers of the library, but rather also internal developers that are maintaining it, looking for better understanding of the binary crate. It's key that developers onboarding into the project have the material to have a swift ramp-up process, to move the project forward.
Once the issue above is stabilized, we can add the following to the test.yml fire:
- name: cargo test --doc
run: cargo test --locked --all-features --doc
The overarching theme with this contribution is: I see CI as an iterative process, and I'm sure we'll be changing it in the future. This acts as a good start, and opens the project to possible issues it may have, that CI now catches (say, dependency issue, as we see running cargo test with -Zminimal-versions).
I see this first PR as not fixing all the issues with the repository, but rather creating the stepping stones to fix them in the next commits. Every commit that includes this PR has been tested with the CI changes, iterating on the process. I'm open to rebasing the commits if needed, but it's useful to see the iteration process to arrive to a CI that fits the project's needs.
I was able to get a successfully configured CI pipeline for the Rune project, and propose additional improvements that we can discuss on the PR for it. I see CI as an iterative process, and I bet it's something we modify sooner than we think. As suggested by the step by step game plan above, next steps include:
loom could potentially fit (and add it to the CI).Lots of work left, really excited about the future!
P.S: Here's the PR link: #35