Skip to content

Typechecking tidying#386

Merged
neersighted merged 2 commits into
python-poetry:mainfrom
dimbleby:typechecking-tidying
Jun 2, 2022
Merged

Typechecking tidying#386
neersighted merged 2 commits into
python-poetry:mainfrom
dimbleby:typechecking-tidying

Conversation

@dimbleby
Copy link
Copy Markdown
Contributor

@dimbleby dimbleby commented Jun 2, 2022

I was just going to tidy up some Any annotations, but in doing so I noticed that nearly all of the code in the Link object is unused. So I removed it.

Did these in separate commits in case it's controversial

@dimbleby dimbleby force-pushed the typechecking-tidying branch 2 times, most recently from af5f41f to 4fbcd8f Compare June 2, 2022 13:55
@dimbleby
Copy link
Copy Markdown
Contributor Author

dimbleby commented Jun 2, 2022

oh, looks like there's one downstream caller that sets the extra parameters (but doesn't care about them... I'll get an MR over there too.

@neersighted
Copy link
Copy Markdown
Member

I think I'm okay with factoring this code out of Link, given it is there to support the legacy PyPI API that we have migrated off of (and our current implementation for compatible repositories was greatly simplified by @abn) -- that being said, I'm curious if Poetry 1.1.x still uses this code and if a newer core may break stable Poetry. I think this highlights the need for a downstream test against stable in addition to the development branch.

@abn
Copy link
Copy Markdown
Member

abn commented Jun 2, 2022

Metadata code should not be removed; this is used to support PEP 658.

see: python-poetry/poetry#5509

@dimbleby dimbleby force-pushed the typechecking-tidying branch from 58b8caf to 022cd48 Compare June 2, 2022 16:13
@dimbleby
Copy link
Copy Markdown
Contributor Author

dimbleby commented Jun 2, 2022

reinstated metadata

@neersighted
Copy link
Copy Markdown
Member

reinstated metadata

python-poetry/poetry#5509 also requires requires_python -- consensus here is that removing interfaces from core is a bad idea with no deprecation period unless they were ephemeral (e.g. only existed in alphas/betas before removal).

That being said, I think we can remove comes_from as we have no plans to use that field as best I can tell, and the types it was originally intended to be used with have been removed or heavily changed.

@dimbleby
Copy link
Copy Markdown
Contributor Author

dimbleby commented Jun 2, 2022

I'm curious if Poetry 1.1.x still uses this code and if a newer core may break stable Poetry

well I'd be all in favour of this project respecting semantic versioning: the next release should be 2.0.0.

But poetry 1.1.13 requires "~1.0.7", so it'll be ok.

Will put requires_python back too: comes_from was always the worst of these...

@dimbleby dimbleby force-pushed the typechecking-tidying branch from 022cd48 to 9a007cc Compare June 2, 2022 16:31
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@neersighted
Copy link
Copy Markdown
Member

I'm curious if Poetry 1.1.x still uses this code and if a newer core may break stable Poetry

well I'd be all in favour of this project respecting semantic versioning: the next release should be 2.0.0.

But poetry 1.1.13 requires "~1.0.7", so it'll be ok.

Will put requires_python back too: comes_from was always the worst of these...

Don't worry, we're talking about that too and trying to come up with a better scheme for both upstream and end-users. I'd still love for you to join Discord and be part of these discussions, since we tend to flesh them out there before posting a design or implementation here 😄

@neersighted neersighted merged commit b09c747 into python-poetry:main Jun 2, 2022
@dimbleby dimbleby deleted the typechecking-tidying branch June 2, 2022 16:56
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