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

initial cut of context support #406

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asuffield
Copy link

No description provided.

conn.go Show resolved Hide resolved
@johnweldon
Copy link
Member

Very nice - thank you.

When you're ready to add the changes to the v3 folder, and you're satisfied it's ready we can make the final approval call.

@firefart
Copy link

firefart commented Feb 6, 2024

any update on this? Having a proper context in the package for timeouts and cancellation would be really nice to have

@asuffield
Copy link
Author

I'm not able to work on it at present (annoying employer restrictions on open source contributions). Please finish it off and ship it if you can.

@firefart
Copy link

firefart commented Feb 7, 2024

I just had a look at the code and I am a bit confused because the same code seems to live in the v3 and in the main directory and past pull requests modify both places. Which one is the correct place to implement the context usage? I guess the root directory is pre v3 and should not be modified?

@johnweldon
Copy link
Member

I just had a look at the code and I am a bit confused because the same code seems to live in the v3 and in the main directory and past pull requests modify both places. Which one is the correct place to implement the context usage? I guess the root directory is pre v3 and should not be modified?

You're right to be confused - we started down the path of modifying both places before we (I) fully understood the Go versioning and module scheme - for now we continue to duplicate the changes between the root and v3.

At some point we'll need to clean up the repository, but haven't taken the time yet.

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.

3 participants