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

static_tests: Add test for Rust code formatting rules #20886

Closed
wants to merge 1 commit into from

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Oct 2, 2024

Contribution description

It is common for projects with Rust code to just enforce the code style suggested by Rust.

This adds a check to the static tests, and

Testing procedure

This PR should fail with a sensible message on the first run, and be green when I push the second commit fixing all the formatting errors (through the provided command).

Issues/PRs references

Thanks @Teufelchen1 for spotting the formatting errors.

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Oct 2, 2024
@chrysn chrysn added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: Rust Area: Rust wrapper Area: tools Area: Supplementary tools and removed Area: tools Area: Supplementary tools labels Oct 2, 2024
@riot-ci
Copy link

riot-ci commented Oct 2, 2024

Murdock results

✔️ PASSED

20880fe static_tests: Add test for Rust code formatting rules

Success Failures Total Runtime
1 0 1 01m:36s

Artifacts

@chrysn
Copy link
Member Author

chrysn commented Oct 2, 2024

Hm, there is a holdup: The static tests are run in a different container than riotbuild, and that doesn't have Rust :-/

Should I just add it there?

@Teufelchen1
Copy link
Contributor

I would say yes. Is there an easy way to measure the extra time this will cost per static-test run?

@chrysn
Copy link
Member Author

chrysn commented Oct 2, 2024

Hard to tell w/o knowing GitHub's container cache strategy, but I think that the time to fetch the container (10s in this case) will grow proportionally with its size in some approximation.

@Teufelchen1
Copy link
Contributor

Lets just do it. How bad can it be? 🙈

chrysn added a commit to chrysn-pull-requests/riotdocker that referenced this pull request Oct 2, 2024
This is needed for the static tests introduced in [20886]; no need for
nightly or target specific tools.

[20886]: RIOT-OS/RIOT#20886
chrysn added a commit to chrysn-pull-requests/riotdocker that referenced this pull request Oct 2, 2024
This is needed for the static tests introduced in [20886]; no need for
nightly or target specific tools.

[20886]: RIOT-OS/RIOT#20886
@chrysn
Copy link
Member Author

chrysn commented Oct 2, 2024

This got shut down by GitHub trying to do NLP and failing miserably, closing this through RIOT-OS/riotdocker#254 … and since I force-pushed to make it trigger again, I can't reopen either.

Anyone ever heard of Forgejo?

Updated PR incoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Rust Area: Rust wrapper Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants