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

POC: Type-safe name canonicalization #7984

Closed
wants to merge 1 commit into from

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Apr 5, 2020

This is a proof of concept for #7974.

I introduced a wrapper function pip._internal.utils.packaging.canonicalize_name() to return a new type CanonicalName, and tried that on FormatControl. Seems to work pretty well, it initially reported quite a few type errors, and I could convert the required hints quite easily by following the error messages.

@uranusjr uranusjr added the skip news Does not need a NEWS file entry (eg: trivial changes) label Apr 5, 2020
@pradyunsg
Copy link
Member

But pypa/packaging#292... :(

@uranusjr
Copy link
Member Author

uranusjr commented Apr 5, 2020

This is just a POC :)

We only need to change the imports (back) with your change merged in.

@pradyunsg
Copy link
Member

Oh come on, why is RTD not reporting status now?

@McSinyx
Copy link
Contributor

McSinyx commented Apr 8, 2020

@pradyunsg, RTD builds fine, could be GitHub issue? On my personal repos there are times where Travis has the same problem (successful build but showed as pending on GH checks).

@pradyunsg
Copy link
Member

Yea, it's probably a case of a GitHub outage overlapping with when the result was posted back. RTD's not a blocker for merging PRs though, so it's fine. :)

@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 May 21, 2020
@pradyunsg
Copy link
Member

FWIW, packaging 20.4 now has the relevant changes included. Whenever we re-vendor that, we'll have to addresss this issue (mostly).

@uranusjr uranusjr closed this May 22, 2020
@uranusjr uranusjr deleted the canonical-name branch May 22, 2020 07:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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
Development

Successfully merging this pull request may close these issues.

4 participants