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

Remove absolute import. #901

Merged
merged 1 commit into from
Jun 20, 2016
Merged

Conversation

Lukasa
Copy link
Sponsor Contributor

@Lukasa Lukasa commented Jun 20, 2016

This absolute import breaks vendorizing urllib3: see kennethreitz/requests#3006.

This absolute import breaks vendorizing urllib3.
@haikuginger
Copy link
Contributor

👍; I don't see a reason to not go ahead and merge.

@shazow
Copy link
Member

shazow commented Jun 20, 2016

Good call, maybe we need somekind of test (integration or otherwise) to avoid this kind of error in the future.

@shazow shazow merged commit 8a6136d into urllib3:master Jun 20, 2016
@Lukasa
Copy link
Sponsor Contributor Author

Lukasa commented Jun 20, 2016

I think we can probably just write a really quick script for it: nothing in the urllib3 directory should have a file with the string "import urllib3" in it.

@haikuginger
Copy link
Contributor

haikuginger commented Jun 20, 2016

Or from urllib3.

@Lukasa
Copy link
Sponsor Contributor Author

Lukasa commented Jun 20, 2016

Good call. It'd probably be pretty easy to write this as a little third-party Python tool that checks for this sort of thing if you were really interested, but otherwise it's basically just a grep.

@haikuginger
Copy link
Contributor

For right now, maybe skip the automation - those constructs are valid within comments/docstrings, so to identify them would be substantially more complex than a plain grep.

I ended up doing grep -r -v '^#' urllib3 | grep -e 'import urllib3' -e 'from urllib3' and did find one additional change that needed to be made, but other than that, the other item it found was just inside a docstring. It's quite likely that there's a way to exclude docstrings as well, but at the moment I don't have the regex magic necessary to make that happen.

Dobatymo pushed a commit to Dobatymo/urllib3 that referenced this pull request Mar 16, 2022
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