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.

Sign up for GitHub

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

Jump to bottom

Enable disallow_untyped_defs for _internal.utils.* #6096

Closed
wants to merge 12 commits into from
Closed

Enable disallow_untyped_defs for _internal.utils.* #6096

wants to merge 12 commits into from

Conversation

mkurnikov
Copy link
Contributor

@mkurnikov mkurnikov commented Dec 21, 2018 •
edited
Loading

Refs #6092.

@mkurnikov mkurnikov changed the title finish types for pip._internal.utils.models, pip._internal.utils.pack… Finish types for pip._internal.utils.models, pip._internal.utils.packaging Dec 21, 2018
@mkurnikov mkurnikov changed the title Finish types for pip._internal.utils.models, pip._internal.utils.packaging [WIP] Enable disallow_untyped_defs for _internal.utils.* Dec 24, 2018
@mkurnikov
Copy link
Contributor Author

Let me know if you want me to split PR into a couple of smaller ones.

@mkurnikov mkurnikov changed the title [WIP] Enable disallow_untyped_defs for _internal.utils.* Enable disallow_untyped_defs for _internal.utils.* Dec 24, 2018
@pradyunsg pradyunsg added the skip news Does not need a NEWS file entry (eg: trivial changes) label Dec 24, 2018
@pradyunsg
Copy link
Member

Let me know if you want me to split PR into a couple of smaller ones.

Nah, this seems small enough -- I do think simplifying the config file to just enable it in utils as a whole would be perfect. :)

@mkurnikov
Copy link
Contributor Author

mkurnikov commented Dec 24, 2018 •
edited
Loading

Done it already in the latest commit. (Oh, sorry, I maybe misunderstood it).

@pradyunsg
Copy link
Member

Nah, this seems right; I'll go through the code changes later though.

@pradyunsg pradyunsg self-requested a review February 6, 2019 02:13
pradyunsg
pradyunsg reviewed Feb 21, 2019
View reviewed changes
src/pip/_internal/utils/ui.py
@@ -240,18 +266,19 @@ class DownloadBlueEmojiProgressBar(BaseDownloadProgressBar, # type: ignore
pass


class DownloadProgressSpinner(WindowsMixin, InterruptibleMixin,
DownloadProgressMixin, WritelnMixin, Spinner):
class DownloadProgressSpinner(BaseProgressIndicator, # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

Why has this changed?

@pradyunsg
Copy link
Member

Could you please update this PR to be based on the current master?

@mkurnikov
Copy link
Contributor Author

Could you first review/merge #6287 and #6282, and then I'll update this and other PR based on latest mypy?

@pradyunsg
Copy link
Member

Sounds about right! :)

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 23, 2019
@pradyunsg
Copy link
Member

pradyunsg commented May 23, 2019 •
edited
Loading

Thanks for this PR @mkurnikov (and all the others)!

I'll be taking the rest of this up sometime myself/keeping them open for first time contributors! I'm closing this PR since it's bitrotten now.

@pradyunsg pradyunsg closed this May 23, 2019
@pradyunsg
Copy link
Member

@mkurnikov Lemme know if you're interested in contributing to pip further. :D

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 22, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pradyunsg pradyunsg pradyunsg left review comments

Assignees
No one assigned
Labels
auto-locked Outdated issues that have been locked by automation needs rebase or merge PR has conflicts with current master skip news Does not need a NEWS file entry (eg: trivial changes)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mkurnikov @pradyunsg @BrownTruck