Conversation
| &self, | ||
| path: &str, | ||
| object: &impl Serialize, | ||
| object: &(impl Serialize + ?Sized), |
There was a problem hiding this comment.
I am not sure what is reason for this change as it compiles before.
There was a problem hiding this comment.
Explained by Gemini:
1. Clippy's Suggestion: Clippy correctly pointed out that passing &Box<RawValue> was inefficient and suggested using &RawValue instead (clippy::ptr_arg).
2. The Problem: RawValue is an unsized type (it's essentially a wrapper around a str). While Box<RawValue> is Sized (because the pointer has a known size), the bare RawValue is not.
3. The Error: When I changed the argument to &RawValue, the compiler complained that RawValue does not implement Sized, and therefore couldn't be passed to the HTTP client methods which were defined as &impl
Serialize.
4. The Fix: Adding + ?Sized (the "widening" or "relaxed" bound) tells the compiler: "This type must implement Serialize, but it's okay if its size isn't known at compile time."
This allowed the BaseHTTPClient to be more flexible, accepting both standard sized types and reference-passed unsized types like RawValue or str.
|
|
||
| let mut software_config = self.config.software.clone().unwrap_or_default(); | ||
| let product_config = software_config.product.get_or_insert_default(); | ||
| let product_config = software_config.product.get_or_insert_with(Default::default); |
There was a problem hiding this comment.
not sure why here it will change it. Previous one looks like specialized method.
There was a problem hiding this comment.
Introduced in Rust 1.83 (and we are aiming at Rust 1.81). I guess we could change the minimal version according to SLES.
| .create(true) | ||
| .truncate(true) | ||
| .write(true) | ||
| .open(not_copy_path) |
There was a problem hiding this comment.
here I would probably use inspect_err instead to make it more readable and easier.
| fn wins_servers(&self) -> zbus::Result<Vec<u32>>; | ||
| } | ||
|
|
||
| #[allow(dead_code)] |
There was a problem hiding this comment.
why it is dead_code when it is used as result?
| } | ||
|
|
||
| #[allow(dead_code)] | ||
| pub type IP6Address = (Vec<u8>, u32, Vec<u8>); |
There was a problem hiding this comment.
The proxies are auto-generated, but the change looks good.
| if let Error::Manager(error) = &self { | ||
| if matches!(error, ManagerError::PendingIssues { issues: _ }) | ||
| || matches!(error, ManagerError::Busy { scopes }) | ||
| if matches!(**error, ManagerError::PendingIssues { issues: _ }) |
There was a problem hiding this comment.
not sure what is reason for **. Also instead of issues: _ I would use { .. }
| use tokio::sync::broadcast; | ||
|
|
||
| #[derive(Debug, thiserror::Error)] | ||
| pub enum Error { |
There was a problem hiding this comment.
that looks kind of strange that ws Error is not used.
| } | ||
| } | ||
|
|
||
| /// Information about possible solution for conflict |
There was a problem hiding this comment.
well, this is not implemented feature for new software stack. So I would at least tag it in git, so we can resurrect when the need arise.
| #[cfg(not(ci))] | ||
| fn test_passwords() { | ||
| let checker = PasswordChecker::default(); | ||
| let checker = PasswordChecker; |
There was a problem hiding this comment.
this looks kind of strange.
| } | ||
|
|
||
| pub fn next(&mut self) -> Result<(), Error> { | ||
| pub fn advance(&mut self) -> Result<(), Error> { |
There was a problem hiding this comment.
this is API change. When I see it in my test run, I am not sure if is good change.
There was a problem hiding this comment.
Well, I guess it is proposing that name because next is part of the iterator's API. So it looks like Progress implements that API but it does not.
| Ok(c) => c, | ||
| Err(e) => { | ||
| let _ = child.kill(); | ||
| let _ = child.wait(); |
There was a problem hiding this comment.
here it duplicate drop method. I am not sure if it is needed?
| /// * `mode`: product mode. Required only if the product has modes. | ||
| pub fn find(&self, id: &str, mode: Option<&str>) -> Result<ProductSpec, Error> { | ||
| let mut mode = mode.clone(); | ||
| pub fn find<'a>(&'a self, id: &str, mut mode: Option<&'a str>) -> Result<ProductSpec, Error> { |
There was a problem hiding this comment.
not sure why it needs lifetimes here when it compile without them.
| )); | ||
| }; | ||
| let Some(login) = credentials.get(0).and_then(Value::as_str) else { | ||
| let Some(login) = credentials.first().and_then(Value::as_str) else { |
There was a problem hiding this comment.
here I also see it in my test run and do not like much this change as later is get(1) and this way it looks inconsistent. Previously it is sequence of get by position.
|
I have decided to replace this PR with #3209, making a clear distinction between automatic changes from Clippy and Gemini. |
2 participants
Problem
Address clippy complains.
Solution
Use Gemini to fix the given issues so we need to go careful in the review process.