wg
wg copied to clipboard
rust-embedded
[RFC] Style checking our libs with rustfmt
Was discussed on the WG meeting on the 2019-01-08, the logs can be of reference.
I am generally :+1: on this plan but I worry about edge cases where the rustfmt style is really bad for embedded, especially (only?) how it handles svd2rust's API with code like
rcc.ahb1enr.modify(|_, w| {
w.crcen()
.enabled()
.ethmacrxen()
.enabled()
.ethmactxen()
.enabled()
.ethmacen()
.enabled()
});
I doubt I'm alone in preferring to have it go register().variant() per line. and the alignment seems totally off as well. I think this could be fixed with a custom rustfmt style for embedded projects, which might be worth looking in to.
On the other hand, I don't think much or any of the code maintained by the WG actually uses svd2rust crates much (only HAL implementations which tend to be in their own orgs), so it might be a total non-issue for us.
Finally much as I'd much prefer having bors just apply rustfmt when it does the merge commit, I don't think this is possible with bors at the moment, so we'd probably just have to make it a Travis check and insist contributors run rustfmt themselves.
@adamgreig Rustfmt formats this correctly and it doesn't look bad. It may be worth it to adopt this style
The question of automating this can now be part of the git hooks using https://github.com/rhysd/cargo-husky
I vote we merge this, and select the option of rejecting a PR if the author hasn't run rustfmt. We can also squash their commits to avoid formatting related noise.
Just to throw another option in the mix, GitHub Actions can trigger a script on push to master that can itself push back to master, so we could have that automatically run rustfmt after each PR is merged.
I'm also happy with having Travis just reject the PR if rustfmt isn't clean, and optionally add a Husky pre-commit hook to remind authors to format code before committing.
This would mean an additional commit for each fmt run? While convenient, the added noise would be unpleasant I think.
You can also push the formatting changes to a PR itself unless the author has unchecked “Allow edits by maintainers”. Then again, I think it is a reasonable assumption that someone who can write Rust code is also capable of running rustfmt.
