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

upgrade to declarative setuptools #1428

Merged

Conversation

graingert
Copy link
Member

@graingert graingert commented Oct 8, 2020

Contributor Checklist:

@graingert graingert marked this pull request as draft October 8, 2020 12:18
@graingert graingert force-pushed the move-more-content-into-static-package-metadata branch 10 times, most recently from 958548a to c33a81b Compare October 8, 2020 14:20
@graingert graingert force-pushed the move-more-content-into-static-package-metadata branch from c33a81b to 7c1ff22 Compare October 8, 2020 15:42
@graingert graingert marked this pull request as ready for review October 8, 2020 15:44
@graingert graingert closed this Oct 8, 2020
@graingert graingert reopened this Oct 8, 2020
@graingert graingert changed the title move more content into static package metadata switch to declarative setuptools Oct 8, 2020
# are not yet set up when this code is executed.
"ext_modules": extensions,
"cmdclass": {"build_ext": my_build_ext},
"extras_require": _EXTRAS_REQUIRE,
Copy link
Member Author

Choose a reason for hiding this comment

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

currently _EXTRAS_REQUIRE is in python to save on duplication eg with twisted[all_non_platform]

@graingert graingert closed this Oct 8, 2020
@graingert graingert reopened this Oct 8, 2020
@graingert graingert changed the title switch to declarative setuptools upgrade to declarative setuptools Oct 8, 2020
@graingert graingert closed this Oct 8, 2020
@graingert graingert reopened this Oct 8, 2020
@graingert
Copy link
Member Author

One concern I have though is that test_mail.py and test_options.py are removed, but the code that they're testing is not. This is probably related to the removal of the notPortedModules list, but it seems like something that should be resolved in a more thorough matter, perhaps in a separate PR.

I think reverting the tests and fixing them is on the table. Or deleting these notPortedModules. I don't think they do any harm in the dist though

@graingert graingert closed this Oct 9, 2020
@graingert graingert reopened this Oct 9, 2020
@wsanchez
Copy link
Member

wsanchez commented Oct 9, 2020

One concern I have though is that test_mail.py and test_options.py are removed, but the code that they're testing is not. This is probably related to the removal of the notPortedModules list, but it seems like something that should be resolved in a more thorough matter, perhaps in a separate PR.

I think reverting the tests and fixing them is on the table. Or deleting these notPortedModules. I don't think they do any harm in the dist though

I agree with @mthuurne that removing them is problematic…

@wsanchez wsanchez closed this Oct 9, 2020
@wsanchez
Copy link
Member

wsanchez commented Oct 9, 2020

WHOAH oops didn't mean to close

@wsanchez wsanchez reopened this Oct 9, 2020
@wsanchez
Copy link
Member

wsanchez commented Oct 9, 2020

I think this is a lot more readable and I like moving the static config back up to the top of the project. I've never really used setup.cfg so I'm not aware of what pitfalls may exist, if any, or what the best way to verify the built result is OK, though I guess that's what pre-releases are for…?

@graingert
Copy link
Member Author

graingert commented Oct 9, 2020

or what the best way to verify the built result is OK, though I guess that's what pre-releases are for…?

a pypi pre-release or a test.pypi.org upload would work. Tox does run this so it mostly works and the wheels seem fine.

you can run python setup.py sdist bdist_wheel and inspect the results

@wsanchez
Copy link
Member

wsanchez commented Oct 9, 2020

a pypi pre-release or a test.pypi.org upload would work. Tox does run this so it mostly works and the wheels seem fine.

I like that.

@rodrigc @glyph How do you feel about giving one of us upload permissions to Twisted on test.pypi.org?

@graingert
Copy link
Member Author

@wsanchez you could rename the setup.cfg[metadata].name key

@rodrigc
Copy link
Contributor

rodrigc commented Oct 9, 2020

@rodrigc @glyph How do you feel about giving one of us upload permissions to Twisted on test.pypi.org?

I can't help you with that. I also am blocked on that: #1423 (comment)

@graingert
Copy link
Member Author

@rodrigc you appear to be soul maintainer of https://test.pypi.org/project/Twisted/ you could add @wsanchez as a maintainer

@rodrigc
Copy link
Contributor

rodrigc commented Oct 9, 2020

Interesting. I don't know when I was granted that access to test.pypi.org. For now I am going to focus on moving forward with the release, before giving additional people access to PyPI resources. If Glyph or Hawkowl want to grant access, I will defer to their wishes.

@glyph
Copy link
Member

glyph commented Oct 9, 2020

@rodrigc You weren't "granted access" - you own the project in its entirety on test.pypi.org; the data there does not derive from PyPI at all.

https://test.pypi.org/project/Twisted/

I didn't even have an account on test.pypi.org — I just made one, so at least I got "glyph" :). Please go ahead and give everybody who needs it access to test PyPI (including me :)).

@rodrigc
Copy link
Contributor

rodrigc commented Oct 9, 2020

@glyph OK, I granted you and @wsanchez access to https://test.pypi.org/ . I couldn't find an account for hawkowl on test.pypi.org to grant access.

@graingert graingert force-pushed the move-more-content-into-static-package-metadata branch 2 times, most recently from 9a52f95 to 16a0299 Compare October 9, 2020 19:22
@graingert graingert force-pushed the move-more-content-into-static-package-metadata branch from 291b1f0 to d50e810 Compare October 10, 2020 09:28
@graingert
Copy link
Member Author

I agree with @mthuurne that removing them is problematic…

I've restored the tests that were "commented out" in src/twisted/python/_setup.py, ported just enough code to get them to run the test setUps and skipped every test that fails

Copy link
Member

@wsanchez wsanchez left a comment

Choose a reason for hiding this comment

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

cool, thanks

@graingert graingert merged commit 1292fad into twisted:trunk Oct 10, 2020
@graingert graingert deleted the move-more-content-into-static-package-metadata branch October 10, 2020 20:38
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

5 participants