Skip to content

bpo-31739: Fixed socket documentation to use with statement for socket examples.#4181

Closed
Fladam wants to merge 1 commit into
python:mainfrom
Fladam:fix-issue-31739
Closed

bpo-31739: Fixed socket documentation to use with statement for socket examples.#4181
Fladam wants to merge 1 commit into
python:mainfrom
Fladam:fix-issue-31739

Conversation

@Fladam

@Fladam Fladam commented Oct 30, 2017

Copy link
Copy Markdown

@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

Comment thread Doc/library/socket.rst
if not data: break
conn.send(data)
with s:
with conn:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indentation is a bit inconsistent here. Please 4-space indentation.

Comment thread Doc/library/socket.rst
# create a raw socket and bind it to the public interface
s = socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_IP)
s.bind((HOST, 0))
# the public network interface

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same there. # the public network interface should start at the same level of indentation as import socket (line 1680)

@bedevere-bot

Copy link
Copy Markdown

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.

Comment thread Doc/library/socket.rst
if not data: break
conn.send(data)
with s:
with conn:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since 3.1 and 2.7 you can put two context managers in one with s, conn statement. Or you could extend the with s bit to include the s.accept call above.

@matrixise matrixise added the docs Documentation in the Doc dir label May 15, 2019
@JulienPalard

Copy link
Copy Markdown
Member

Hi @Fladam thanks for helping enhancing the Python documentation! Some core developers made reviews, could you please take a look at them?

@AlexWaygood

Copy link
Copy Markdown
Member

I'm closing this PR, as it has been over 4 years and the reviews have not been addressed.

@Fladam, if you're still interested in working on this issue, please feel free to submit a new PR 🙂

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants