-
Notifications
You must be signed in to change notification settings - Fork 76
Optimize the workflow for python CI #989
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
Optimize the workflow for python CI #989
Conversation
Pull Request Test Coverage Report for Build 17138567941Details
💛 - Coveralls |
c31f0d3 to
300c5c0
Compare
spacebear21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK, same comments as the Dart PR with a few more suggestions specific to this one.
| - name: "Build wheel" | ||
| run: python setup.py bdist_wheel --verbose | ||
| run: python -m build --wheel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this change do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should in theory do nothing different, there were simply warnings that using setup.py directly was a deprecated method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| build-backend = "setuptools.build_meta" | ||
|
|
||
| [project] | ||
| name = "payjoin" | ||
| description = "The Python language bindings for the Payjoin Dev Kit" | ||
| readme = "README.md" | ||
| requires-python = ">=3.8" | ||
| license = "MIT" | ||
| dynamic = ["version"] | ||
|
|
||
| [tool.setuptools] | ||
| packages = ["payjoin"] | ||
| package-dir = { "payjoin" = "src/payjoin" } | ||
| include-package-data = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the gist of these update? Do they perhaps belong in a separate commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can split them up, this was due to the setup.py usage being deprecated vs calling python -m build --wheel
c362f8a to
92c1c78
Compare
92c1c78 to
5c95feb
Compare
|
Look at those SAVINGS! For comparison a build from before the one above is an extreme but we get on average ~20min in savings! was it worth the entire day I've been stuck in this workflow doom loop? I say YES! |
5c95feb to
e460446
Compare
spacebear21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK, awesome gainz
This is a simplification for the python build and test workflow for linux and macos. This also updates some of the setup.py to use the more up to date `python -m build` method
setup.py is deprecated for use directly asa build tool
e460446 to
cff82bb
Compare
|
latest commit just fixed a readme typo |
I moved this code incorrectly in payjoin#989 to a general binding build script but others not using a debian based system immediately ran into problems running this apt based package manager on their machine. It was removed in payjoin#1000 from the binding script but This probably should just be returned to the workflow it was originally.
I moved this code incorrectly in payjoin#989 to a general binding build script but others not using a debian based system immediately ran into problems running this apt based package manager on their machine. It was removed in payjoin#1000 from the binding script but This probably should just be returned to the workflow it was originally.
I moved this code incorrectly in payjoin#989 to a general binding build script but others not using a debian based system immediately ran into problems running this apt based package manager on their machine. It was removed in payjoin#1000 from the binding script but This probably should just be returned to the workflow it was originally.
This is a simplification for the python build and test workflow for linux and macos.
This also updates some of the setup.py to use the more up to date
python -m buildmethodThis is part 2 to supersede #980
TODO
look at the comments addressing migrating away from pip #864 (comment)actually since this change is touching more than just the workflow lets move it to a follow-upPull Request Checklist
Please confirm the following before requesting review: