ADR: add rust linting & formatting ADR proposal#80
Conversation
Signed-off-by: Elena Gantner <elena.gantner@mercedes-benz.com>
|
Related suggestion: Once we agree on these rules they can be put into the CICD repository, to enable re-use. Furhtermore the CICD repo can offer a check to validate them and leave comments via reviewdog, similar to what we did in the CDA here |
| The goal is to formalize the usage of linting rules for rust using clippy and | ||
| set common codestyle rules applied through rustfmt. | ||
| This will help having a single style across the different components which | ||
| improves understandability and readability across the project context. |
There was a problem hiding this comment.
| improves understandability and readability across the project context. | |
| improves understandability and readability across the project context and hence facilitates easier maintainability long-term. |
?
| # or `.saturating_sub(...)` to avoid unexpected runtime behavior or panics. | ||
| arithmetic_side_effects = "deny" | ||
| ## lints related to readability of code | ||
| # enforce that references are cloned via eg. `Arc::clone` instead of `.clone()` |
There was a problem hiding this comment.
what's the advantage of enforcing this? maybe add a short rationale here?
There was a problem hiding this comment.
Making it explicit that a reference is cloned, not the data that is referenced to (1:1 the reasoning clippy gives for the lint)
I can add this a short oneline reason though, thanks :)
| - Neutral: Might cause additional efforts when bringing in parts of existing | ||
| codebases into the context of OpenSOVD, but enforces those codebases to be on | ||
| par with the OpenSOVD expectations of clean & modern code. |
There was a problem hiding this comment.
Well, I would argue here that it is inevitable that contributions from outside will adhere/be compliant to our coding guidelines (which we define for a reason) and hence are obliged to invest such efforts. As a result, such matter should not be a criterion here.
There was a problem hiding this comment.
I'm removing that section, as I agree with your sentiment.
| ### Defining a new set of common rules | ||
|
|
||
| With this option the idea is to take a look at what the CDA currently does and | ||
| set things that people deem to restrictive to optional with appropriate |
There was a problem hiding this comment.
| set things that people deem to restrictive to optional with appropriate | |
| set things that people deem too restrictive to optional with appropriate |
?
or what was meant exactly here?
| set things that people deem to restrictive to optional with appropriate | |
| set things that people deem to restrict to optional with appropriate |
There was a problem hiding this comment.
oh the first one, thanks! :)
|
|
||
| - Good: potentially easier for new projects built upon existing codebases to | ||
| adapt to | ||
| - Negative: Could potentially lead to more fragmentation in regards to codestyle |
There was a problem hiding this comment.
| - Negative: Could potentially lead to more fragmentation in regards to codestyle | |
| - Negative: Could potentially lead to more fragmentation with regards to codestyle |
Signed-off-by: Elena Gantner <elena.gantner@mercedes-benz.com>
|
Thanks for the feedback @stmuench :) |
3 participants
Summary
Add an ADR regarding rust linter & formatting rules.
Outcome TBD before merging.
Notes for Reviewers
Feel free to add suggestions or propose a third option to be decided in the architecture meeting.