-
Notifications
You must be signed in to change notification settings - Fork 703
[core split] Layer: retry #6961
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.
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
Conversation
| #[cfg(feature = "layers-mime-guess")] | ||
| pub use self::mime_guess::MimeGuessLayer; | ||
|
|
||
| #[path = "../../../layers/retry/src/lib.rs"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, please check our tracking issue #6829 again, this is not we want.
| let buf = reader.read().await?; | ||
| self.reader = Some(reader); | ||
| self.args.range_mut().advance(buf.len() as u64); | ||
| let mut range = self.args.range(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems not needed, please don't touch our layer's impl.
|
Hi, just curious, are you using a code agent for this PR? If so, are you using the prompts I provided in the tracking issue? Which tool and LLM are you using? |
@Xuanwo Sorry for missing the fact that the tracking issue already provided a complete prompt. I will revert the two current PRs (#6964 and #6961), and then rework the changes by following the prompt you provided and submit a new set of commits accordingly. For transparency, the LLM tool I used was Codex. |
Thanks for the clarification and explanation. That makes sense — I see why migrating the logging, retry, and timeout layers together is preferable given the test coupling. Thanks for pointing this out and for the reference to #7053. I appreciate the feedback and will align with this direction in future contributions. Looking forward to contributing again when there’s a good fit. |
Closes #6947
This PR splits the layer
retryout ofcoreas part of the core-split effort and follows the tracking issue #6829.Summary