Skip to content
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.

Sign up for GitHub

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

Jump to bottom

remove the RequireAddressValidation callback from the Config #4253

Merged
merged 1 commit into from
Jan 23, 2024
Merged

remove the RequireAddressValidation callback from the Config #4253

merged 1 commit into from
Jan 23, 2024

Conversation

marten-seemann
Copy link
Member

Depends on #4248.

@marten-seemann marten-seemann force-pushed the remove-require-address-validation-callback branch 2 times, most recently from b9fbe71 to 212d204 Compare January 19, 2024 09:02
@marten-seemann marten-seemann linked an issue Jan 19, 2024 that may be closed by this pull request
remove the RequireAddressValidation callback #4252
Closed
@marten-seemann marten-seemann force-pushed the remove-require-address-validation-callback branch from 212d204 to ec22f77 Compare January 19, 2024 10:26
@codecov Codecov
Copy link

codecov bot commented Jan 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (892851e) 84.08% compared to head (78e43cd) 84.08%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4253      +/-   ##
==========================================
- Coverage   84.08%   84.08%   -0.01%     
==========================================
  Files         150      150              
  Lines       15431    15425       -6     
==========================================
- Hits        12975    12969       -6     
  Misses       1953     1953              
  Partials      503      503

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

@marten-seemann marten-seemann added this to the v0.42 milestone Jan 21, 2024
@marten-seemann marten-seemann mentioned this pull request Jan 21, 2024
Open
sukunrt
sukunrt approved these changes Jan 22, 2024
View reviewed changes
Copy link
Collaborator

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

This specific change looks good. But I think you need to defer this block that decrements the handshake count: https://github.com/quic-go/quic-go/blob/remove-require-address-validation-callback/server.go#L734-L745

There are a couple of returns just above that block.

@marten-seemann
Copy link
Member Author

This specific change looks good. But I think you need to defer this block that decrements the handshake count: https://github.com/quic-go/quic-go/blob/remove-require-address-validation-callback/server.go#L734-L745

There are a couple of returns just above that block.

Done in #4248.

@marten-seemann marten-seemann force-pushed the remove-require-address-validation-callback branch from ec22f77 to 78e43cd Compare January 23, 2024 05:05
@marten-seemann marten-seemann changed the base branch from retry-config to master January 23, 2024 05:05
@marten-seemann marten-seemann merged commit a2cf43d into master Jan 23, 2024
18 checks passed
@marten-seemann marten-seemann deleted the remove-require-address-validation-callback branch January 25, 2024 07:00
mgjeong pushed a commit to mgjeong/quic-go that referenced this pull request Feb 13, 2024
@marten-seemann @mgjeong
remove the RequireAddressValidation callback from the Config (quic-go…
4ba1dde 
…#4253)
@linted linted mentioned this pull request Mar 18, 2024
Closed
@github-actions github-actions bot mentioned this pull request Mar 18, 2024
Closed
@MSevey MSevey mentioned this pull request Apr 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sukunrt sukunrt sukunrt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.42
Development

Successfully merging this pull request may close these issues.

remove the RequireAddressValidation callback
2 participants
@marten-seemann @sukunrt