Skip to content

Conversation

@philipch07
Copy link
Contributor

@philipch07 philipch07 commented Oct 31, 2025

Description

  • Updates references in ack_timer to RFC 9260
  • Adds a clamp for the max delay according to RFC 9260

Note that this doesn't currently allow for the delay to be configurable, but the clamp is there in case the default value changes to ensure correctness.

Reference issue

Resolves #428.

@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

❌ Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.85%. Comparing base (a69c6a2) to head (086aa13).

Files with missing lines Patch % Lines
ack_timer.go 69.23% 2 Missing and 2 partials ⚠️

❌ Your patch check has failed because the patch coverage (69.23%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #429      +/-   ##
==========================================
+ Coverage   83.84%   83.85%   +0.01%     
==========================================
  Files          51       51              
  Lines        3448     3457       +9     
==========================================
+ Hits         2891     2899       +8     
  Misses        417      417              
- Partials      140      141       +1     
Flag Coverage Δ
go 83.85% <69.23%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@philipch07 philipch07 force-pushed the pch07/update-ack-timer-9260 branch 3 times, most recently from 655e193 to cf92bf8 Compare October 31, 2025 06:24
@philipch07 philipch07 changed the title Update ack_timer to use goroutines and RFC 9260 Update ack_timer to RFC 9260 Oct 31, 2025
@philipch07 philipch07 force-pushed the pch07/update-ack-timer-9260 branch from cf92bf8 to 43ec894 Compare October 31, 2025 06:39
@JoeTurki JoeTurki requested a review from Copilot November 2, 2025 05: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 ackTimer to support configurable SACK (Selective Acknowledgement) delays per RFC 9260 requirements. The changes improve RFC compliance, fix spelling errors, enhance defensive programming around timer state management, and update documentation to reflect the correct RFC version.

  • Added configurable SACK delay with proper validation bounds (0, 500ms]
  • Improved defensive checks on pending counter to prevent underflow
  • Updated documentation and corrected spelling mistakes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@philipch07 philipch07 force-pushed the pch07/update-ack-timer-9260 branch from 43ec894 to 086aa13 Compare November 2, 2025 05:59
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.

Update ack_timer to RFC 9260

2 participants