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

Update v2 to use pyproject.toml #7287

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Update v2 to use pyproject.toml #7287

merged 2 commits into from
Sep 27, 2022

Conversation

stealthycoin
Copy link
Contributor

@stealthycoin stealthycoin commented Sep 21, 2022

This PR is to consolidate our build tooling around pyproject.toml. We were running into issues finding a version of setuptools that would work on python 3.8 through 3.10 for the sdist building PR: #7262. We decided to take the opportunity to move away from using setuptools entirely and to re-write our build tooling around flit, while at the same time moving from setup.py/setup.cfg fully over to pyproject.toml.
Setuptools has a lot of legacy functionality, and is constantly deprecating old behaviors and evolving. Because of this it has been the cause of a lot of issues for us in the past as behavior changes out from under us. Flit is new and very minimal. Our goal in the change is to have a stable core to our build system that we won't have to build around so defensively.

@stealthycoin stealthycoin changed the title Pyproject Update v2 to use pyproject.toml Sep 21, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2022

Codecov Report

Merging #7287 (d4fae65) into v2 (a97df2b) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##               v2    #7287      +/-   ##
==========================================
+ Coverage   93.70%   93.71%   +0.01%     
==========================================
  Files         351      351              
  Lines       36171    36171              
  Branches     5202     5202              
==========================================
+ Hits        33895    33899       +4     
+ Misses       1652     1648       -4     
  Partials      624      624              
Impacted Files Coverage Δ
awscli/s3transfer/tasks.py 95.00% <0.00%> (+3.33%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks good. Just put down some initial comments. I plan to take a deeper look between our current sdist/wheel and the ones as part of this PR in the next iteration.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
backends/pep517.py Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
tests/backends/test_pep517.py Show resolved Hide resolved
tests/backends/test_pep517.py Outdated Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
@stealthycoin stealthycoin force-pushed the pyproject branch 7 times, most recently from 562f5c1 to b4d17d8 Compare September 25, 2022 17:25
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

It's looking good. I'm still diving through it, but just wanted to send some comments back now so we can keep making progress on it while I go through it.

.changes/next-release/feature-packaging-11704.json Outdated Show resolved Hide resolved
backends/pep517.py Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
tests/backends/test_pep517.py Show resolved Hide resolved
backends/pep517.py Show resolved Hide resolved
backends/pep517.py Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Finished making my pass through all of this. It is looking really good. I just had some more comments to add on to my previous review.

tests/backends/test_pep517.py Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
backends/pep517.py Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
backends/pep517.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Nice. This looks good. I just had a few more comments. It would also be good if we can expand in the PR description why we chose flit explicitly over setuptools and the benefits we get. Also feel free to squash all of the commits to what the final commit will be.

.changes/next-release/feature-packaging-11704.json Outdated Show resolved Hide resolved
tests/backends/test_pep517.py Show resolved Hide resolved
tests/backends/test_pep517.py Outdated Show resolved Hide resolved
This change removes both the setup.py and setup.cfg files in favor of
using the pyproject.toml. In addition we remove our build dependencies
on both seuptools and wheel, instead using flit as our build backend.
Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks good! 🚢

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