Skip to content

Comments

Modernize code base#131

Merged
sdispater merged 3 commits intomasterfrom
modernize-codebase
Mar 3, 2021
Merged

Modernize code base#131
sdispater merged 3 commits intomasterfrom
modernize-codebase

Conversation

@sdispater
Copy link
Member

@sdispater sdispater commented Feb 3, 2021

This PR modernizes the code by doing the following:

  • Remove the PYTHON2.7/3.5 compatibility layers
  • Use proper type annotations

@sdispater sdispater force-pushed the modernize-codebase branch 2 times, most recently from b3186bd to 26509f9 Compare February 5, 2021 13:37
@sdispater sdispater marked this pull request as ready for review February 12, 2021 15:19
@sdispater sdispater requested a review from a team February 12, 2021 15:19
finswimmer
finswimmer requested changes Feb 14, 2021
View reviewed changes
@sdispater
Copy link
Member Author

sdispater commented Feb 26, 2021

@finswimmer Let me know if you still see something that needs changing.

@sdispater
Copy link
Member Author

sdispater commented Mar 3, 2021

I am gonna go ahead and merge this.

The import issue has already been fixed on Poetry's side (python-poetry/poetry#3696) and the fix will be available in the next 1.1.5 release.

@sdispater sdispater merged commit b103125 into master Mar 3, 2021
@sdispater sdispater deleted the modernize-codebase branch March 3, 2021 10:29
nickray
nickray reviewed Mar 4, 2021
View reviewed changes
poetry/core/pyproject/__init__.py
@@ -1,6 +0,0 @@
from poetry.core.pyproject.exceptions import PyProjectException
Copy link

@nickray nickray Mar 4, 2021

Choose a reason for hiding this comment

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

This removal breaks https://github.com/python-poetry/poetry/blob/6eaa2984d9fb51a837c4ab11643f4dd9d7828d4d/poetry/console/commands/config.py#L14 (current master).

nickray
nickray reviewed Mar 4, 2021
View reviewed changes
poetry/core/semver/__init__.py
from .patterns import TILDE_PEP440_CONSTRAINT
from .patterns import X_CONSTRAINT
from .version import Version
from .version_constraint import VersionConstraint
Copy link

@nickray nickray Mar 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This removal of the VersionConstraint import breaks https://github.com/python-poetry/poetry/blob/6eaa2984d9fb51a837c4ab11643f4dd9d7828d4d/poetry/repositories/repository.py#L5 (current master).

@nickray
Copy link

nickray commented Mar 4, 2021 •
edited
Loading

@sdispater: Besides the type annotations and Python version housecleaning, multiple changes in this PR modify the public API of poetry-core. Technically, these are semver-breaking changes. Is this intended? Is there a corresponding PR to poetry dealing with the fallout already?

@sdispater
Copy link
Member Author

sdispater commented Mar 9, 2021

@nickray Thanks for sharing you concerns.

poetry-core is a PEP-517 build backend and is not indented to be used as a library – except by Poetry itself – for now. The only public API that matters at the moment are the PEP-517 hooks provided by poetry.core.masonry.api.

There is currently a work in progress branch (https://github.com/python-poetry/poetry/compare/modern-core) on Poetry's side to update the imports and code to account for these changes in poetry-core.

julienmalard pushed a commit to julienmalard/poetry-core that referenced this pull request Mar 26, 2021
@sdispater @julienmalard
Modernize code base (python-poetry#131)
ccf2575 
* Update CI files for Python 3.6+ only

* Modernize codebase

* Reorganize imports to improve performance
@sdispater sdispater mentioned this pull request Mar 30, 2021
Merged
@Dekker1 Dekker1 mentioned this pull request Sep 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@finswimmer finswimmer finswimmer requested changes

+1 more reviewer

@nickray nickray nickray left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sdispater @nickray @finswimmer