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 some deprecated code #3948

Merged
merged 10 commits into from Jun 19, 2023
Merged

Conversation

abravalheri
Copy link
Contributor

Summary of changes

Remove code that has been deprecated for a while:

Closes

Pull Request Checklist

@abravalheri abravalheri changed the title Remove deprecated code Remove some deprecated code Jun 12, 2023
@abravalheri

This comment was marked as outdated.

@@ -123,7 +123,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python }}${{ matrix.dev }}
python-version: 3.11-dev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This code is maintained upstream in jaraco/skeleton. I'm not sure what it even means to have matrix.python and matrix.dev in this section. Probably these lines simply need to be removed upstream. But that still doesn't address the concern that building against the default Python causes problems for Setuptools. Probably this change deserves a comment linking to a bug as to why it's here, because by pinning a version, it creates cruft that someone will have to revisit in the future (maybe just link to the comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Jason.

I changed the PR to avoid modifying the workflow.

I believe the error in the CI is a bootstrapping problem, when upgrading the setuptools installation from source. The first time egg_info runs (via build_meta) in an environment that comes with an old version setuptools pre-installed, it will read the previous .dist-info/entry-points.txt and try to import egg_info.warn_depends_obsolete, resulting in error.

To avoid this error, the new commit will simply leave a stub behind which can then be removed in future versions.

@abravalheri
Copy link
Contributor Author

abravalheri commented Jun 12, 2023

@jaraco would you be OK with these removals or should we postpone them?

I also have #3951 open to remove old py*compat modules.

Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@@ -123,7 +123,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python }}${{ matrix.dev }}
python-version: 3.11-dev
Copy link
Member

Choose a reason for hiding this comment

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

This code is maintained upstream in jaraco/skeleton. I'm not sure what it even means to have matrix.python and matrix.dev in this section. Probably these lines simply need to be removed upstream. But that still doesn't address the concern that building against the default Python causes problems for Setuptools. Probably this change deserves a comment linking to a bug as to why it's here, because by pinning a version, it creates cruft that someone will have to revisit in the future (maybe just link to the comment).

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.

None yet

2 participants