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

Consistently name threadlocal and contextvars helper functions #238

Closed
MarkusH opened this issue Jan 12, 2020 · 4 comments · Fixed by #240
Closed

Consistently name threadlocal and contextvars helper functions #238

MarkusH opened this issue Jan 12, 2020 · 4 comments · Fixed by #240

Comments

@MarkusH
Copy link
Contributor

MarkusH commented Jan 12, 2020

The threadlocal and contextvars integrations use a different naming for their functions to bind, unbind, merge and clear the context. This came up as part of #236 and e3a6213.

I'd like to propose the following function names:

  • contextvars
    • merge_contextvars_context
    • clear_contextvars_context
    • bind_to_contextvars_context
    • unbind_from_contextvars_context
  • threadlocal
    • merge_threadlocal_context
    • clear_threadlocal_context
    • bind_to_threadlocal_context
    • unbind_from_threadlocal_context -- doesn't exist but should probably be added
MarkusH referenced this issue Jan 12, 2020
The two `context` in merge_context_local_context may look weird but they refer
to two different contexts...
@hynek
Copy link
Owner

hynek commented Jan 13, 2020

Hmm that seems a little bit too repetitive and wordy. 🤔

Maybe the best way forward is to just drop the context from merge and be done with it? Typing bind_to_contextvars_context() whenever you do something doesn't seem like a pragmatic API. :)

@hynek
Copy link
Owner

hynek commented Jan 14, 2020

  • unbind_from_threadlocal_context -- doesn't exist but should probably be added

lol #239

@hynek
Copy link
Owner

hynek commented Jan 18, 2020

Yeah I'm just gonna drop all the contexts unless someone wants to stop me.

@MarkusH
Copy link
Contributor Author

MarkusH commented Jan 18, 2020

Yeah I'm just gonna drop all the contexts unless someone wants to stop me.

Sounds good to me 👍

hynek added a commit that referenced this issue Jan 18, 2020
@hynek hynek mentioned this issue Jan 18, 2020
hynek added a commit that referenced this issue Jan 18, 2020
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 a pull request may close this issue.

2 participants