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: Add workflow to ensure Rust code is consistently formatted #95

Closed
wants to merge 79 commits into from

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Nov 14, 2024

Rather than leaving formatting decisions up to every developer, enforce them consistently with rustfmt. To do so, add a workflow that checks that the code has been properly formatted, with "cargo fmt --check".

The objective of creating this workflow early, while there are few formatting reports to address, is to avoid having a huge commit at a later time to fix formatting throughout the repository.

daniel-noland and others added 30 commits August 29, 2024 16:03
This is a total mess but it is just for validation at this point
This reverts commit e2ce55f.
Flake lock file updates:

• Updated input 'flake-utils':
    'github:numtide/flake-utils/b1d9ab70662946ef0850d488da1c9019f3a9752a?narHash=sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ%3D' (2024-03-11)
  → 'github:numtide/flake-utils/c1dfcf08411b08f6b8615f7d8971a2bfa81d5e8a?narHash=sha256-X6rJYSESBVr3hBoH0WbKE5KvhPU5bloyZ2L4K60/fPQ%3D' (2024-09-17)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/8b4061fd60ccc3b3f44b73faa7c983eacf7a6f7b?narHash=sha256-gT%2BrUDbw%2BTQuszQEzMUJWTW7QYtccZ5xxWmKOSrPvEw%3D' (2024-09-03)
  → 'github:NixOS/nixpkgs/294eb5975def0caa718fca92dc5a9d656ae392a9?narHash=sha256-WO6Lmbn37PlamY2fDg3B187THkSKU/W01z8SxoIqJd0%3D' (2024-09-18)
The version spec for dependencies was sloppy (failed to specify minor and patch levels)

Keep in mind that in cargo, dependency of type

```toml
foo = "x.y.z"
```

means that `foo` must resolve with version `x'.y'.z'` where `x' == x`, `y' >= y`, and `z' >= z`

and that the default dependency resolver will maximize `y'` and `z'`.

That is, cargo assumes that projects are following semver and that you want the latest package which meets your desired API.

If you really needed a specific version you could say

```toml
foo = "=x.y.z"
```

which constrains the resolver
Flake lock file updates:

• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/294eb5975def0caa718fca92dc5a9d656ae392a9?narHash=sha256-WO6Lmbn37PlamY2fDg3B187THkSKU/W01z8SxoIqJd0%3D' (2024-09-18)
  → 'github:NixOS/nixpkgs/a1d92660c6b3b7c26fb883500a80ea9d33321be2?narHash=sha256-V5LpfdHyQkUF7RfOaDPrZDP%2Boqz88lTJrMT1%2BstXNwo%3D' (2024-09-20)
Bumps the dev-dependencies group with 1 update in the / directory: [cc](https://github.com/rust-lang/cc-rs).


Updates `cc` from 1.1.18 to 1.1.21
- [Release notes](https://github.com/rust-lang/cc-rs/releases)
- [Changelog](https://github.com/rust-lang/cc-rs/blob/main/CHANGELOG.md)
- [Commits](rust-lang/cc-rs@cc-v1.1.18...cc-v1.1.21)

---
updated-dependencies:
- dependency-name: cc
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: dev-dependencies
...

Signed-off-by: dependabot[bot] <[email protected]>
Make sure we only run the docs-related workflow on docs-related changes
only, both for Pull Requests and when pushing new code. Also rename the
workflow to make it explicit it builds and deploys _docs_.

Link: https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#example-including-paths
Signed-off-by: Quentin Monnet <[email protected]>
Avoid running the workflow when pushing on other branches. We already
run it for Pull Requests.

Signed-off-by: Quentin Monnet <[email protected]>
Cancel an ongoing docs build when the Pull Request (or the HEAD, when
building following a push) is updated. No need to run concurrent builds
for a same PR.

Signed-off-by: Quentin Monnet <[email protected]>
This directory has not been used in a while.

It was for a prior round of quick scratch work and was checked in accidentally in the first place.
Bumps the dev-dependencies group with 1 update in the / directory: [libc](https://github.com/rust-lang/libc).


Updates `libc` from 0.2.158 to 0.2.159
- [Release notes](https://github.com/rust-lang/libc/releases)
- [Changelog](https://github.com/rust-lang/libc/blob/0.2.159/CHANGELOG.md)
- [Commits](rust-lang/libc@0.2.158...0.2.159)

---
updated-dependencies:
- dependency-name: libc
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: dev-dependencies
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps the dev-dependencies group with 1 update in the / directory: [cc](https://github.com/rust-lang/cc-rs).


Updates `cc` from 1.1.21 to 1.1.22
- [Release notes](https://github.com/rust-lang/cc-rs/releases)
- [Changelog](https://github.com/rust-lang/cc-rs/blob/main/CHANGELOG.md)
- [Commits](rust-lang/cc-rs@cc-v1.1.21...cc-v1.1.22)

---
updated-dependencies:
- dependency-name: cc
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: dev-dependencies
...

Signed-off-by: dependabot[bot] <[email protected]>
Flake lock file updates:

• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/a1d92660c6b3b7c26fb883500a80ea9d33321be2?narHash=sha256-V5LpfdHyQkUF7RfOaDPrZDP%2Boqz88lTJrMT1%2BstXNwo%3D' (2024-09-20)
  → 'github:NixOS/nixpkgs/b5b2fecd0cadd82ef107c9583018f381ae70f222?narHash=sha256-k6YxGj08voz9NvuKExojiGXAVd69M8COtqWSKr6sQS4%3D' (2024-09-28)
When listing files affecting the workflow to build/publish
documentation, we forgot to list flake.lock. Let's fix.

Fixes: 4c15025 ("CI: Rename docs build workflow, restrict to docs changes")
Signed-off-by: Quentin Monnet <[email protected]>
Bumps the dev-dependencies group with 1 update in the / directory: [cc](https://github.com/rust-lang/cc-rs).


Updates `cc` from 1.1.22 to 1.1.23
- [Release notes](https://github.com/rust-lang/cc-rs/releases)
- [Changelog](https://github.com/rust-lang/cc-rs/blob/main/CHANGELOG.md)
- [Commits](rust-lang/cc-rs@cc-v1.1.22...cc-v1.1.23)

---
updated-dependencies:
- dependency-name: cc
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: dev-dependencies
...

Signed-off-by: dependabot[bot] <[email protected]>
daniel-noland and others added 15 commits November 11, 2024 21:19
From man gitignore:

    A leading "**" followed by a slash means match in all directories.
    For example, "**/foo" matches file or directory "foo" anywhere, the
    same as pattern "foo".

In other words, we don't need "/target/**" in the .gitignore if we
already have "**/target/**". Same applies to "/result*". Let's remove
the redundant rules.

Also sort the file alphabetically (as per Vim's ":%s").

Signed-off-by: Quentin Monnet <[email protected]>
We have set a version for just in the CI workflow but don't use it.
Remove it.

Signed-off-by: Quentin Monnet <[email protected]>
Make the README more welcoming to readers who don't know the project by
adding a quick intro and linking to the docs. Also add a warning to
state that the project is not usable, yet.

Signed-off-by: Quentin Monnet <[email protected]>
Let Clippy run in pedantic mode, and fix some of the issue it reports in
the repository.

Fixes left aside:

  - Expansion of dependencies in "use" directives
  - Use of casts introducing new std::ptr (as I'm not sure we want to
    rely more on the standard library)
  - Use of backquotes around "EtherType" (not the variable) in a comment

Signed-off-by: Quentin Monnet <[email protected]>
In TX queue's "configure()", we do not otherwise consume the "config"
object that we get as an argument. We can simply reuse it to craft the
return TxQueue object rather than cloning the struct - just like we do
for RxQueue.

Found in Clippy's reports (Clippy suggested to pass "config" by
reference).

Signed-off-by: Quentin Monnet <[email protected]>
Make everything owned by a single team. The idea is to be able to have
auto-assignment for reviews on Pull Requests. We can use finer-grained
ownership later if necessary.

Signed-off-by: Quentin Monnet <[email protected]>
Make workflow description files more consistent:

- Add a comment to describe the workflow at the top of the file, for
  workflows that didn't have one yet
- Use consistent options for tmate options on "workflow_dispatch"
- Use a consistent order for trigger events ("on:")
- Use a consistent order for workflow permissions
- Use a blank line between top-level sections
- Remove trailing blank lines at the end of files

The objective is to make it easier to compare the different workflow
files visually.

No functional change.

Signed-off-by: Quentin Monnet <[email protected]>
Rather than running the dev.yml and sterile.yml workflows everytime
developers push to any branch, run them for "pull_target" events. This
makes it easier to integrate with Pull Requests (for example for setting
required tests) and avoids running unnecessary workflows when branches
are pushed with no immediate intention to open a PR.

Keep running on "push", but for "main" branch only, to run the workflows
on the new "main" when PRs get merged.

Signed-off-by: Quentin Monnet <[email protected]>
In a previous commit I tried to clean up the README file, but I broke
the link to rustup's website. Let's fix it.

Fixes: aa8a163 ("README.md: Clean-up minor issues, add link to Fabric's docs")
Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet qmonnet added the ci Continuous Integration label Nov 14, 2024
@qmonnet qmonnet force-pushed the pr/qmonnet/rustfmt branch 2 times, most recently from 3d40472 to 68ae03a Compare November 14, 2024 16:47
Format files consistently using rustfmt.

Signed-off-by: Quentin Monnet <[email protected]>
Rather than leaving formatting decisions up to every developer, enforce
them consistently with rustfmt. To do so, add a workflow that checks
that the code has been properly formatted, with "cargo fmt --check".

Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet qmonnet added the dont-merge Do not merge this Pull Request label Nov 14, 2024
@qmonnet
Copy link
Member Author

qmonnet commented Nov 14, 2024

[Hold off until #92 is complete]

@qmonnet qmonnet marked this pull request as ready for review November 14, 2024 16:50
@qmonnet qmonnet requested a review from a team as a code owner November 14, 2024 16:50
@qmonnet qmonnet requested review from daniel-noland and removed request for a team November 14, 2024 16:50
@qmonnet qmonnet closed this Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration dont-merge Do not merge this Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants