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.

Sign up for GitHub

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

Jump to bottom

Simplify Result<()> uses and implement a checkpatch.pl check for it #1128

Open
ojeda opened this issue Nov 16, 2024 · 4 comments
Open

Simplify Result<()> uses and implement a checkpatch.pl check for it #1128

ojeda opened this issue Nov 16, 2024 · 4 comments
Labels
easy Expected to be an easy issue to resolve. good first issue Good for newcomers • lib Related to the `rust/` library.

Comments

@ojeda
Copy link
Member

ojeda commented Nov 16, 2024 •
edited
Loading

There are 5 cases of Result<()> at the time of writing -- can they be simplified?

Additionally, implement a checkpatch.pl check for it. This should be done in a different patch.

Please note that the checkpatch.pl maintainers will need to agree to the change.

This requires submitting a proper patch to the LKML and the Rust for Linux mailing list. Please recall to test your changes (including generating the documentation if changed, running the Rust doctests if changed, etc.), to use a proper title for the commit, to sign your commit under the Developer's Certificate of Origin and to add a Suggested-by: tag and a Link: tag to this issue. Please see https://rust-for-linux.com/contributing for details.

Please take this issue only if you are new to the kernel development process and you would like to use it as a test to submit your first patch to the kernel. Please do not take it if you do not plan to make other contributions to the kernel.
@ojeda ojeda added good first issue Good for newcomers easy Expected to be an easy issue to resolve. labels Nov 16, 2024
@ojeda ojeda changed the title Simplify Result<()> uses Simplify Result<()> uses and implement a checkpatch.pl check for it Nov 17, 2024
@ojeda
Copy link
Member Author

ojeda commented Nov 18, 2024

v1: https://lore.kernel.org/rust-for-linux/[email protected]/
v2: https://lore.kernel.org/rust-for-linux/[email protected]/
net: https://lore.kernel.org/rust-for-linux/[email protected]/

kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Nov 18, 2024
@weirdsmiley
net: phy: qt2025: simplify Result<()> in probe return
99c2c6d 
probe returns a `Result<()>` type, which can be simplified as `Result`,
due to default type parameters being unit `()` and `Error` types. This
maintains a consistent usage of `Result` throughout codebase.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
Signed-off-by: NipaLocal <nipa@local>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Nov 18, 2024
@weirdsmiley
net: phy: qt2025: simplify Result<()> in probe return
e35eb56 
probe returns a `Result<()>` type, which can be simplified as `Result`,
due to default type parameters being unit `()` and `Error` types. This
maintains a consistent usage of `Result` throughout codebase.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
Reviewed-by: Miguel Ojeda <[email protected]>
Signed-off-by: NipaLocal <nipa@local>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Nov 18, 2024
@weirdsmiley
net: phy: qt2025: simplify Result<()> in probe return
e39e899 
probe returns a `Result<()>` type, which can be simplified as `Result`,
due to default type parameters being unit `()` and `Error` types. This
maintains a consistent usage of `Result` throughout codebase.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
Reviewed-by: Miguel Ojeda <[email protected]>
Signed-off-by: NipaLocal <nipa@local>
kuba-moo pushed a commit to linux-netdev/testing that referenced this issue Nov 19, 2024
@weirdsmiley
net: phy: qt2025: simplify Result<()> in probe return
7d6e8e4 
probe returns a `Result<()>` type, which can be simplified as `Result`,
due to default type parameters being unit `()` and `Error` types. This
maintains a consistent usage of `Result` throughout codebase.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
Reviewed-by: Miguel Ojeda <[email protected]>
Signed-off-by: NipaLocal <nipa@local>
klarasm pushed a commit to klarasm/linux that referenced this issue Nov 19, 2024
@weirdsmiley @axboe
rust: block: simplify Result<()> in validate_block_size return
a3f143c 
`Result` is used in place of `Result<()>` because the default type
parameters are unit `()` and `Error` types, which are automatically
inferred. Thus keep the usage consistent throughout codebase.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
Reviewed-by: Miguel Ojeda <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Nov 21, 2024
@weirdsmiley @intel-lab-lkp
net: phy: qt2025: simplify Result<()> in probe return
c38ef81 
probe returns a `Result<()>` type, which can be simplified as `Result`,
due to default type parameters being unit `()` and `Error` types. This
maintains a consistent usage of `Result` throughout codebase.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Nov 21, 2024
@weirdsmiley @intel-lab-lkp
rust: simplify Result<()> uses
f181955 
This patch replaces `Result<()>` with `Result`.

Suggested-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux#1128
Signed-off-by: Manas <[email protected]>
@ojeda ojeda added the • lib Related to the `rust/` library. label Nov 25, 2024
@weirdsmiley
Copy link

@ojeda I have the second patch for checkpatch.pl ready. But I have been holding it, as I haven't resolved the signature issue yet. Should I send it regardless for other feedbacks?

@ojeda
Copy link
Member Author

ojeda commented Nov 28, 2024

Sure, that could be useful -- but I would recommend making it clear that you are working on that, e.g. with a "Not-yet-Signed-off-by" tag or similar, or a note below the first --- line.

Having said that, I think we are still waiting on the checkpatch.pl maintainer(s) for a similar issue, so I am not sure how much time they have currently: https://lore.kernel.org/rust-for-linux/CANiq72noG77qLF1WcRu4qJNboyqs9Xvvu_ZmjVN8qitK_MNWnw@mail.gmail.com/

Thanks!

@ojeda
Copy link
Member Author

ojeda commented Nov 30, 2024

https://lore.kernel.org/rust-for-linux/20241128-add-checkpatch-result-simplify-v1-1-973603191d71@iiitd.ac.in/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
easy Expected to be an easy issue to resolve. good first issue Good for newcomers • lib Related to the `rust/` library.
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ojeda @weirdsmiley