Skip to content

[doc] Update cookbook example for socket-based logging in a production sett… (GH-98922)#98922

Merged
vsajip merged 3 commits into
python:mainfrom
vsajip:cookbook-update
Nov 1, 2022
Merged

[doc] Update cookbook example for socket-based logging in a production sett… (GH-98922)#98922
vsajip merged 3 commits into
python:mainfrom
vsajip:cookbook-update

Conversation

@vsajip

@vsajip vsajip commented Oct 31, 2022

Copy link
Copy Markdown
Member

…ing.

@vsajip vsajip added docs Documentation in the Doc dir skip issue skip news labels Oct 31, 2022
@vsajip vsajip requested a review from CAM-Gerlach October 31, 2022 19:21
@vsajip vsajip self-assigned this Oct 31, 2022
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated

@CAM-Gerlach CAM-Gerlach left a comment

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.

Looks good overall, just some minor syntax and textual suggestions, and one higher-level comment:

Now that these files are an integral part of the recipe here, maybe better to just host them here alongside it rather than in a personal account on an external site, and provide a direct download link (using :download:)?

Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@vsajip

vsajip commented Nov 1, 2022

Copy link
Copy Markdown
Member Author

Thanks @slateny and @CAM-Gerlach for your feedback. I've updated the PR in line with your suggestions.

Now that these files are an integral part of the recipe here, maybe better to just host them here alongside it rather than in a personal account on an external site, and provide a direct download link (using :download:)?

I did think about that, but I'm not sure there's a precedent for hosting a mini-project like this in the documentation. I didn't want to inline these files in the documentation because of (a) the length of some of them and (b) it's easier to download them all in one ZIP from the Gist, for those who want to try them out and maybe change some things. And since Gists are a part of GitHub, I didn't think of it as an external site. It is a personal account, of course - I don't know if organization-level Gists are a thing.

@CAM-Gerlach CAM-Gerlach left a comment

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.

Looking good; I just made suggestions to single-source the long Gist URL instead of maintaining it multiple places embedded in the text, making it easier to update and harder to get out of sync (plus being less disruptive of source text flow). I also fixed a typo'd comma left by one of my previous suggestions. Other than that, LGTM!

Comment thread Doc/howto/logging-cookbook.rst
Comment thread Doc/howto/logging-cookbook.rst Outdated
Comment thread Doc/howto/logging-cookbook.rst Outdated
@CAM-Gerlach

Copy link
Copy Markdown
Member

@vsajip I see, that makes sense. It still feels a bit odd for a section of the doc to essentially be documentation for an external demo project, but it is a cookbook after all and its not like they come with the ingredients included 😄

Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@vsajip vsajip changed the title [doc] Update cookbook example for socket-based logging in a production sett… [doc] Update cookbook example for socket-based logging in a production sett… (GH-98922) Nov 1, 2022
@vsajip vsajip merged commit 50a9b03 into python:main Nov 1, 2022
@vsajip vsajip deleted the cookbook-update branch November 1, 2022 23:00
@vsajip vsajip added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Nov 1, 2022
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @vsajip for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @vsajip for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 1, 2022
…n sett… (pythonGH-98922)

(cherry picked from commit 50a9b03)

Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bedevere-bot

Copy link
Copy Markdown

GH-98980 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Nov 1, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 1, 2022
…n sett… (pythonGH-98922)

(cherry picked from commit 50a9b03)

Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bedevere-bot

Copy link
Copy Markdown

GH-98981 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 1, 2022
vsajip added a commit that referenced this pull request Nov 1, 2022
…oduction sett… (GH-98922) (GH-98980)

Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
vsajip added a commit that referenced this pull request Nov 1, 2022
…oduction sett… (GH-98922) (GH-98981)

Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants