Skip to content

Conversation

@Peponks9
Copy link
Contributor

@Peponks9 Peponks9 commented Sep 4, 2025

Refactor all instances of Box::pin with core::pin::pin! in the time.rs file. The changes ensure stack-pinning of futures for better performance while maintaining the same functionality.

Changes Made

  • Updated send_after and send_interval functions to use pin! for futures and async blocks.
  • Added let bindings for pinned values to resolve compilation errors related to temporary borrowing.
  • Ensured the code compiles without issues and preserves the original behavior.

Files Changed
time.rs

Testing

  • Verified compilation with cargo check.
  • All tests by running cargo test still passing.

Closes #50
cc @MegaRedHand

@Peponks9 Peponks9 changed the title refactor: replace Box::pin with `core::pin::pin! refactor: replace Box::pin with core::pin::pin! Sep 4, 2025
Copy link
Contributor

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Left some comments about style.

Copy link
Contributor Author

@Peponks9 Peponks9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the feedback @MegaRedHand, changes made to align with comments

@MegaRedHand MegaRedHand requested a review from Copilot September 5, 2025 12:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the time.rs file to replace Box::pin with core::pin::pin! macro for stack-pinning futures, which improves performance by avoiding heap allocation while maintaining the same functionality.

  • Replaced all Box::pin calls with core::pin::pin! macro
  • Added explicit let bindings for pinned futures to resolve compilation issues with temporary borrowing
  • Simplified nested async block structures for better readability

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Peponks9 Peponks9 force-pushed the fix-issue-50 branch 2 times, most recently from 1632451 to 169f76c Compare September 5, 2025 17:18
@Peponks9
Copy link
Contributor Author

Peponks9 commented Sep 5, 2025

Hi @MegaRedHand, all set with signed commits. I had some trouble with signing keys but resolved now. Thanks

@fedacking fedacking merged commit d68258d into lambdaclass:main Sep 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace Box::pin calls with core::pin::pin! macro

3 participants