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

ci: fix wheel building #352

Merged
merged 16 commits into from
Feb 3, 2025
Merged

ci: fix wheel building #352

merged 16 commits into from
Feb 3, 2025

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Jan 27, 2025

you can see the final working test in https://github.com/pyapp-kit/psygnal/actions/runs/13092786487/job/36531555427#step:4:71

CI/CD Workflow Updates:

  • .github/workflows/pypi.yml: Reorganized the build and deployment steps, including renaming jobs, using a new action for building and inspecting Python packages, and consolidating the publish steps.

Dependency Updates:

  • pyproject.toml: Moved the dev dependencies to the optional-dependencies section, updated the test dependencies with specific versions, and added new dependencies under testqt. [1] [2]

Recursion Handling Improvements:

  • src/psygnal/_signal.py: Introduced a fixed recursion limit of 300 to avoid segfaults in mypyc-compiled programs, added checks for recursion depth in the emit_fast and _run_emit_loop methods, and ensured the recursion depth is decremented in a finally block. [1] [2] [3] [4]

@tlambert03 tlambert03 marked this pull request as draft January 27, 2025 15:58
Copy link

codspeed-hq bot commented Jan 27, 2025

CodSpeed Performance Report

Merging #352 will not alter performance

Comparing tlambert03:fix-wheels (9431fba) with main (0f6e607)

Summary

✅ 67 untouched benchmarks

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0f6e607) to head (9431fba).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #352   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         2116      2120    +4     
=========================================
+ Hits          2116      2120    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 75 to 76
# - name: 🚢 Publish to PyPI
# uses: pypa/gh-action-pypi-publish@release/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you upload here instead of have separate steep to collect all wheels anbd upload only if all platfrom buildt successfully?

Copy link
Member Author

Choose a reason for hiding this comment

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

simplicity 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

But you may then upload to pypi broken release that was revealed during build wheel.

Copy link
Member Author

Choose a reason for hiding this comment

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

well, I'd call it a "partial" release. it is actually pretty easy to upload more wheels if only some of them fail to build. In any case, this whole thing is draft, and I'm actively working on it. Nothing you see here at the moment should be considered something I've thought hard about. please hang on a moment. I'll let you know when it's ready for review if you'd like to look

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, short of uncommenting out the pypi publish step, i think this is now better. let me know if it's following all the best practices you would suggest

@tlambert03 tlambert03 marked this pull request as ready for review February 1, 2025 21:54
@tlambert03 tlambert03 enabled auto-merge (squash) February 3, 2025 13:50
@tlambert03 tlambert03 merged commit e951f3f into pyapp-kit:main Feb 3, 2025
48 checks passed
@tlambert03 tlambert03 deleted the fix-wheels branch February 3, 2025 13:55
@tlambert03 tlambert03 added the tests Tests & CI label Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests & CI
Development

Successfully merging this pull request may close these issues.

2 participants