Repository navigation
apply coding style - #19
Conversation
|
In my opinion needed for #1, as the kernel seems strict about those styles. |
|
Do wee need the |
alex
left a comment
There was a problem hiding this comment.
In addition, we should not be reformatting rust/shlex/ which is a 3p package.
| @@ -0,0 +1,4 @@ | |||
| max_width = 80 | |||
| brace_style = "AlwaysNextLine" | |||
| tab_spaces = 8 | |||
There was a problem hiding this comment.
Is there a reason to use a non-standard rustfmt.toml?
There was a problem hiding this comment.
Yes, the coding style defined in Documentation/process/coding-style.rst
There was a problem hiding this comment.
Hmm, so the idea is that the Rust style should follow the C style? My intuition is that it'd be more valuable to follow the Rust ecosystem defaults -- the kernel needed it's own C style guide because there's no uniform C style. But Rust mostly has one, and we ought to follow that.
But I'm interested in the opinions of folks like @ojeda who have more upstream experience.
There was a problem hiding this comment.
The coding style there was indeed intended for C. So I don't think we need to care too much about most of the rules in coding-style.rst. And it is valuable to take the chance to start fresh and follow standard Rust conventions as much as possible.
However, some points can/will be argued that they should be shared among all languages where possible:
- The column width may be contentious. Linus recently allowed to go up to 100 if needed, so there is a precedent on moving these limits up (mainly backed on the fact that we have way bigger, better screens nowadays). On the other hand, I recently tried to raise
.clang-formatto 100 to help the tool a bit and the proposal was met with fair opposition since 80 is still supposed to be the limit. - The tab vs. space (and in particular size) is the bigger one and it also affects the column width. For scripts, it seems the choice is quite varied (e.g. there are Perl and Python scripts using tabs, 2 spaces, 4 spaces, spaces-for-half-indentation...), so there is precedent too. On the other hand, if Rust ends up being the first new language accepted into the kernel (i.e. for drivers and general code), then it is likely that formatting will be a discussion topic and I expect people to argue for tabs to be consistent with C. It is not unlikely the Cargo vs. Kbuild/direct-
rustcthat will likely take place for similar reasons.
Linus et. al. are flexible on style matters, so I think it will come down to what the majority of maintainers feel like it is the best option.
There was a problem hiding this comment.
So Just keeping max_width = 100
And removing everything else?
There was a problem hiding this comment.
That sounds good to me.
And thanks for looking at this!
There was a problem hiding this comment.
By the way, isn't max_width = 100 the default?
Yes, please, all files should have an SPDX ID (the ones that don't, like those in |
Signed-off-by: Finn Behrens <me@kloenk.de>
commented
Oct 5, 2020
I just saw that that is not yet an stable feature of rustfmt. Do we still want the check for it? |
commented
Oct 5, 2020
Ah, I was answering the first question (i.e. we need the header). Concerning automatic checking, it'd be nice to enable it, but let's wait until it hits stable. I'd leave a comment to remind us to enable it when the time comes. |
Add a rustfmt.toml config and run cargo fmt on the codebase.
Taken from
Documentation/process/coding-style.rst