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

Add Additional Context Around Python Signal Handling on iOS #121881

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

michaeleisel
Copy link

@michaeleisel michaeleisel commented Jul 16, 2024

My understanding of this stuff, particularly on the Python side, is somewhat shallow, but from a discussion with @freakboy3742 it appears we can use crash reporters with Python signal handling overrides.


📚 Documentation preview 📚: https://cpython-previews--121881.org.readthedocs.build/

Copy link

cpython-cla-bot bot commented Jul 16, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Jul 16, 2024
@michaeleisel
Copy link
Author

Is there a better way to test the rendering of the .rst file than with rst2html.py?

@freakboy3742
Copy link
Contributor

Is there a better way to test the rendering of the .rst file than with rst2html.py?

There is - when a PR includes documentation changes, the PR body is automatically updated to include a preview rendering.

Alternatively, you can render locally using Sphinx. The CPython developer guide has a section on building the docs locally

Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

This needs line breaks; documentation should be wrapped at 80 characters.

I'm also not convinced this is the best location for this comment; it breaks the bullet-point flow of "things that need to be done" (including, but not limited to the punctuation inconsistency).

I'd suggest moving this comment out of the bullet point flow, an into a paragraph following the discussion of "your apps bundle location" (i.e., a paragraph in "item 10", but not in the "things to do" bullet points.

I think the text itself also needs some modification so that it is more directly actionable.

   .. note::

      Python's default signal handlers will catch the UNIX signals ``SIGPIPE``, 
      ``SIGXFZ``, ``SIGXFSZ``, and ``SIGINT``. These default handlers should be 
      able to co-exist with most third-party iOS crash reporting libraries; but 
      if you experience problems, you can omit the default handlers as long as 
      you manage these signals yourself.

@bedevere-app
Copy link

bedevere-app bot commented Jul 16, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes docs Documentation in the Doc dir needs backport to 3.13 bugs and security fixes skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants