From: Kent Gibson <warthog618@gmail.com>
To: Erik Schilling <erik.schilling@linaro.org>
Cc: Linux-GPIO <linux-gpio@vger.kernel.org>,
Viresh Kumar <viresh.kumar@linaro.org>
Subject: Re: [PATCH libgpiod 4/4] bindings: rust: clippy: silence false-positive on iterator
Date: Fri, 30 Jun 2023 17:08:07 +0800 [thread overview]
Message-ID: <ZJ6bd8+oDbyX06rp@sol> (raw)
In-Reply-To: <20230629-clippy-v1-4-9ff088713c54@linaro.org>
For future reference, the subject line should be "[libgpiod][PATCH...",
as per the README.
Makes it easier to filter visually, if nothing else.
On Thu, Jun 29, 2023 at 01:09:02PM +0200, Erik Schilling wrote:
> This was fixed, but it is not in stable yet.
>
This is not a good checkin comment.
State what the problem is and how the patch addresses it.
Same applies to other patches in the series - but I have other comments
on this one, so raising it here.
> Tested build on x86_64, armv7hf, aarch64.
>
When you say "Tested build", do you mean just compile/clippy, or have you
actually run tests?
Either way, not sure if this should go in the checkin comment - it is
generally implied by the Signed-off that you've tested it to your
satisfaction.
No problem with a more detailed description of how you've tested in
the cover letter.
> Reported-by: Kent Gibson <warthog618@gmail.com>
> Link: https://lore.kernel.org/r/20230612154055.56556-1-warthog618@gmail.com
> Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
> ---
> bindings/rust/libgpiod/src/event_buffer.rs | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs
> index b79e9ea..2e4bfd3 100644
> --- a/bindings/rust/libgpiod/src/event_buffer.rs
> +++ b/bindings/rust/libgpiod/src/event_buffer.rs
> @@ -54,6 +54,9 @@ impl<'a> Iterator for Events<'a> {
> }
>
> fn next(&mut self) -> Option<Self::Item> {
> + // clippy false-positive, fixed in next clippy release:
> + // https://github.com/rust-lang/rust-clippy/issues/9820
> + #[allow(clippy::iter_nth_zero)]
> self.nth(0)
> }
> }
>
Specify the release in absolute terms, not "next clippy release".
Other than those nits, I'm good with the actual changes in the series
(checked with clippy and running tests on a variety of 32 and 64bit
platforms and compiler versions back to 1.60)
(I am seeing this one test failure on arm32, but that doesn't seem to be related
to this patch:
---- request_config::verify::default stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
left: `OperationFailed(RequestConfigGetConsumer, Errno { code: 2, description: Some("No such file or directory") })`,
right: `OperationFailed(RequestConfigGetConsumer, Errno { code: 0, description: Some("Success") })`', libgpiod/tests/request_config.rs:18:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Not sure if that is a genuine bug or something funky in my build.)
Cheers,
Kent.
next prev parent reply other threads:[~2023-06-30 9:08 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-29 11:08 [PATCH libgpiod 0/4] bindings: rust: clippy: fixed warnings Erik Schilling
2023-06-29 11:08 ` [PATCH libgpiod 1/4] bindings: rust: clippy: drop unnecessary casts Erik Schilling
2023-06-29 11:09 ` [PATCH libgpiod 2/4] bindings: rust: clippy: silence false-positives on casts Erik Schilling
2023-06-29 11:09 ` [PATCH libgpiod 3/4] bindings: rust: clippy: drop unneeded conversions Erik Schilling
2023-06-29 11:09 ` [PATCH libgpiod 4/4] bindings: rust: clippy: silence false-positive on iterator Erik Schilling
2023-06-30 9:08 ` Kent Gibson [this message]
2023-06-30 10:05 ` Erik Schilling
2023-06-30 10:19 ` Kent Gibson
2023-06-30 10:46 ` Erik Schilling
2023-06-30 10:50 ` Kent Gibson
2023-06-30 10:53 ` Erik Schilling
2023-07-01 13:10 ` Kent Gibson
2023-06-30 6:08 ` [PATCH libgpiod 0/4] bindings: rust: clippy: fixed warnings Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZJ6bd8+oDbyX06rp@sol \
--to=warthog618@gmail.com \
--cc=erik.schilling@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).