-
Notifications
You must be signed in to change notification settings - Fork 387
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
chore: add packaging without publish to CI #692
base: master
Are you sure you want to change the base?
Conversation
b323ddc
to
5d2ae01
Compare
At the moment, this appears to be failing on unrelated issues. See #691 |
I merged #691 (thank you for that!), so this is ready to be rebased / finish out... |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #692 +/- ##
==========================================
- Coverage 94.78% 94.71% -0.08%
==========================================
Files 57 57
Lines 8339 8339
==========================================
- Hits 7904 7898 -6
- Misses 435 441 +6 see 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
ffdc808
to
63edb1c
Compare
The force push is to get rid of the merge 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.
Thank you!
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.
So this looks fine, but it's also a heavy copy-paste of https://github.com/python-zk/kazoo/blob/1c0c4535c9c82092d94ce36fb7a0be4c43483bb4/.github/workflows/release.yml
The problem will be that if changes happen here or there, they may not be reflected to the other copy.
Could we instead chain the actions such that the steps are DRY'd up... one workflow does the build, the other workflow does the publish, and can trigger the build as part of it's initial steps?
I'm not an actions expert, but a quick google makes it look like this might be possible?
https://docs.github.com/en/actions/using-workflows/reusing-workflows
- name: Set up Python 3.10 | ||
uses: actions/setup-python@v2 | ||
with: | ||
python-version: "3.10" |
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 wonder if we should pin this not here but instead using .python-version
file so that other tooling can use it?
https://github.com/actions/setup-python#basic-usage
Also, now that 3.11 dropped, we should probably use that rather than 3.10?
@jeffwidman Re-thinking about this, it seems we should actually publish "dev" version to the TestPyPI repository (using a version name like Concerning Python 3.11 vs Python 3.10, I agree this is something that we can change once we add a pipeline for the 3.11 version of Python (we "only" have testing for up to 3.10 right now). On my side, even if it is not perfect, I am fine going forward with the current state of the PR and doing the changes I mentioned above in another PR unless, of course, @bringhurst is interested in doing it on his side. |
I think it's probably ok to commit this as-is. It's not perfect, but it gets the job done.
I was looking at the |
Totally fine to punt on publishing to TestPyPI and AFAICT, it should be straightforward to wire up actions such that we've got one workflow that handles all builds, and then a second workflow takes the output from first and actually publishes it... |
628b030
to
333e05b
Compare
3e6091c
to
64e4afb
Compare
Upload to TestPyPI after some sucessful tests Only keep 2 GA workflows, with all the funny things it means
64e4afb
to
9cf8322
Compare
Why is this needed?
CI currently does not exercise packaging for pypi.
Proposed Changes
Does this PR introduce any breaking change?
No.