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.

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

feat(quic): remove QUIC draft-29 version support #5759

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tesol2y090
Copy link

Description

Remove support support_draft_version field from QUIC protocol.

#3395

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@tesol2y090 tesol2y090 marked this pull request as ready for review December 24, 2024 09:05
@elenaf9 elenaf9 changed the title feat(quic): remove QUICk draft version feat(quic): remove QUIC draft-29 version support Dec 31, 2024
Copy link
Contributor

mergify bot commented Dec 31, 2024

This pull request has merge conflicts. Could you please resolve them @tesol2y090? 🙏

Copy link
Contributor

@elenaf9 elenaf9 left a comment

Choose a reason for hiding this comment

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

Thanks @tesol2y090!

I think we should first deprecate support for draft-29 before removing it completely, see Step 1 in #3395 (comment).

@tesol2y090
Copy link
Author

Thanks @tesol2y090!

I think we should first deprecate support for draft-29 before removing it completely, see Step 1 in #3395 (comment).

What is the best way to deprecate support for draft-29, i thought remove the field is ok.

@elenaf9
Copy link
Contributor

elenaf9 commented Dec 31, 2024

Thanks @tesol2y090!
I think we should first deprecate support for draft-29 before removing it completely, see Step 1 in #3395 (comment).

What is the best way to deprecate support for draft-29, i thought remove the field is ok.

Rust offers the #[deprecated(note = "...")] attribute, which will give the user a warning (used e.g. in #3747).

To deprecate support of draft-29 (which is opt-in anyway), we only need to add this attribute to libp2p_quic::Config::support_draft_29.

Removing draft-29 and all related logic (what this PR is doing) can then be done in a follow-up PR after the next release of libp2p-quic.

@tesol2y090
Copy link
Author

Thanks @tesol2y090!
I think we should first deprecate support for draft-29 before removing it completely, see Step 1 in #3395 (comment).

What is the best way to deprecate support for draft-29, i thought remove the field is ok.

Rust offers the #[deprecated(note = "...")] attribute, which will give the user a warning (used e.g. in #3747).

To deprecate support of draft-29 (which is opt-in anyway), we only need to add this attribute to libp2p_quic::Config::support_draft_29.

Removing draft-29 and all related logic (what this PR is doing) can then be done in a follow-up PR after the next release of libp2p-quic.

Thank you @elenaf9 . I just created another #5786 about deprecate support_draft_29

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.

2 participants