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

Switch to private module names #785

Merged
merged 5 commits into from
Jan 28, 2020
Merged

Switch to private module names #785

merged 5 commits into from
Jan 28, 2020

Conversation

florimondmanca
Copy link
Member

Fixes #772

Still need to confirm…

  • References in docs

@florimondmanca florimondmanca added the refactor Issues and PRs related to code refactoring label Jan 20, 2020
@florimondmanca florimondmanca changed the title Private modules Switch to private module names Jan 21, 2020
@tomchristie
Copy link
Member

Fantastic! I think we might also rejig how the imports are arranged, but I think we should probably leave that to a follow-up PR.

(Ie. drop __all__ from the top-level. In cases where we're importing lots of classes from a module consider using eg. from ._exceptions import * in our top-level __init.py, with any Public API exceptions listed in __all__ in the _exceptions.py module. I think it'd be a lot clearer that listing all the import exceptions in a seperate place from where we're defining them.)

How do we want to approach this all release-wise/timing-wise?

Presumably at least both this and #782 in a 0.12.0 release?

@florimondmanca
Copy link
Member Author

Presumably at least both this and #782 in a 0.12.0 release?

Yup, that sounds good to me!

@florimondmanca florimondmanca requested a review from a team January 21, 2020 11:39
@florimondmanca
Copy link
Member Author

Alright, I updated the docs -- only a few references to internal modules in logs, but otherwise nothing indicating that we've got anything missing at the top-level. This should be ready for review! I'll issue a follow-up for reorganizing imports.

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

Good stuff, this'll be a really nice bit of tightening up!
I'm happy for this to go in whenever you are, @florimondmanca.
(Essentially whenever we're at a point where we're confident we want to start pressing ahead on a 0.12.0)

from httpx.content_streams import encode
from httpx.dispatch.base import AsyncDispatcher
from httpx.utils import format_form_param
from httpx._config import CertTypes, TimeoutTypes, VerifyTypes
Copy link
Member

Choose a reason for hiding this comment

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

Incidentally, it looks like we've erronously left the signature as send(request, verify, cert, timeout) in a bunch of these and other test cases, rather than switching to send(request, timeout) on the dispatch interface.

We're missing those because we're not type checking our test cases.

Not related to this PR, so doesn't neccessarily need to change here, but this helps highlight it, since we're pulling in more private API that we might otherwise expect.

@florimondmanca florimondmanca added this to the v0.12 milestone Jan 23, 2020
@tomchristie
Copy link
Member

What do we reckon then? Hit "Go" on this one now, and then start drawing up whatever we want in a 0.12 release?

@florimondmanca
Copy link
Member Author

Yup, I think we’re ready for getting 0.12 together. :)

@tomchristie
Copy link
Member

Okay, let's roll this one now, then.

@tomchristie tomchristie merged commit 82dc6f3 into master Jan 28, 2020
@tomchristie tomchristie mentioned this pull request Mar 5, 2020
@dalf dalf mentioned this pull request Apr 12, 2020
@florimondmanca florimondmanca deleted the private-modules branch May 24, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues and PRs related to code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Private module names
2 participants