Skip to content

bpo-11233: Create availability directive for documentation#9692

Merged
vstinner merged 8 commits into
python:masterfrom
csabella:bpo11233
Oct 12, 2018
Merged

bpo-11233: Create availability directive for documentation#9692
vstinner merged 8 commits into
python:masterfrom
csabella:bpo11233

Conversation

@csabella

@csabella csabella commented Oct 4, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Original patch by Georg Brandl.

https://bugs.python.org/issue11233

vstinner
vstinner requested changes Oct 10, 2018
View reviewed changes

@vstinner vstinner 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.

I like the general approach, but there are a few issues:

  • "make html" generates 3 warnings

/home/vstinner/prog/python/master/Doc/library/os.rst:1111: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/vstinner/prog/python/master/Doc/library/os.rst:1179: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/vstinner/prog/python/master/Doc/library/os.rst:3555: WARNING: Explicit markup ends without a blank line; unexpected unindent.

  • "Availability: Unix." is used outside os.rst: asyncio, signal, time, etc. documentation.

  • Because the availability is used in multiple files, maybe the definition of availability should be described somewhere else? Sorry I have no better proposal, so maybe allos.rst is fine.

@bedevere-bot

bedevere-bot commented Oct 10, 2018

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.

@csabella
csabella requested review from a team, 1st1, asvetlov and gpshead as code owners October 11, 2018 11:33
@csabella

csabella commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

@vstinner

  • I fixed the 'make html' warnings. Sorry that I missed those the first time.
  • Based on the comment about other sources, I also added the availability directive to the other sources. I did it in its own commit so it could be reversed if it's too much.
  • As far as where to store the definition of availability, I looked for a global 'notes'-type page to add it to, but didn't find one. The closest concept that I could find is maybe 'CPython Implementation Detail', but that doesn't have a note defining it. For now, I've moved it to the library intro page. It doesn't flow well with the opening paragraphs, so maybe it should be its own page at the end of the TOC?

vstinner
vstinner reviewed Oct 11, 2018
View reviewed changes

@vstinner vstinner 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.

Hum, I would prefer that the macro doesn't add a trailing dot. I prefer to give more freedom to the user of the macro to decide how to render the text.

Comment thread Doc/library/allos.rst Outdated
Comment thread Doc/tools/extensions/pyspecific.py Outdated
@vstinner
vstinner merged commit 2d6097d into python:master Oct 12, 2018
@vstinner

vstinner commented Oct 12, 2018

Copy link
Copy Markdown
Member

Thanks @csabella! I checked the full PR and it seems like all "Availability" lines now ends with a dot :-) It's more consistent. For the exact location of the definition of Availability, if someone complains, we can now easily change it, since it has a reST label. The whole PR LGTM, I merged it.

I don't want it to be backported. It seems too intrusive to backport it.

@1st1

1st1 commented Oct 12, 2018

Copy link
Copy Markdown
Member

a post-commit LGTM from me.

@1st1

1st1 commented Oct 12, 2018

Copy link
Copy Markdown
Member

BTW, I think this PR should be backported to 3.7, otherwise we might have unnecessary conflicts when backporting docs updates to 3.7.

@vstinner

vstinner commented Oct 12, 2018

Copy link
Copy Markdown
Member

BTW, I think this PR should be backported to 3.7, otherwise we might have unnecessary conflicts when backporting docs updates to 3.7.

If you want to avoid conflicts, in that case, should we also apply it to 3.6 and 2.7?

@1st1

1st1 commented Oct 12, 2018

Copy link
Copy Markdown
Member

We rarely backport doc changes to 2.7 and 3.6. IMO 3.7 would be good enough.

@csabella
csabella deleted the bpo11233 branch October 12, 2018 21:13
@csabella

csabella commented Oct 12, 2018

Copy link
Copy Markdown
Contributor Author

Do you want me to cherry pick to 3.7 or is Miss Islington able to do it post-merge?

@miss-islington

miss-islington commented Oct 12, 2018

Copy link
Copy Markdown
Contributor

Thanks @csabella for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Oct 12, 2018

Copy link
Copy Markdown
Contributor

Sorry, @csabella and @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 2d6097d027e0dd3debbabc702aa9c98d94ba32a3 3.7

@1st1

1st1 commented Oct 12, 2018

Copy link
Copy Markdown
Member

Do you want me to cherry pick to 3.7 or is Miss Islington able to do it post-merge?

Apparently she can't :( Could you please open a PR for 3.7 branch?

csabella added a commit to csabella/cpython that referenced this pull request Oct 12, 2018
@csabella
[3.7] bpo-11233: Create availability directive for documentation (pyt…
b3ae23f 
…honGH-9692)

 Replace "Availability: xxx" with ".. availability:: xxx" in the doc.
 Original patch by Georg Brandl.

 Co-Authored-By: Georg Brandl <georg@python.org>
 (cherry picked from commit 2d6097d)

 Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
@bedevere-bot

bedevere-bot commented Oct 12, 2018

Copy link
Copy Markdown

GH-9830 is a backport of this pull request to the 3.7 branch.

vstinner pushed a commit that referenced this pull request Oct 15, 2018
@csabella @vstinner
[3.7] bpo-11233: Create availability directive for documentation (GH-…
b248a8c 
…9692) (GH-9830)

Replace "Availability: xxx" with ".. availability:: xxx" in the doc.
 Original patch by Georg Brandl.

 Co-Authored-By: Georg Brandl <georg@python.org>
 (cherry picked from commit 2d6097d)

 Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
@sandrotosi sandrotosi mentioned this pull request Aug 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner requested changes
@1st1 1st1 Awaiting requested review from 1st1
@asvetlov asvetlov Awaiting requested review from asvetlov
@gpshead gpshead Awaiting requested review from gpshead

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@csabella @bedevere-bot @vstinner @1st1 @miss-islington @the-knights-who-say-ni