Skip to content

/ cpython

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

Jump to bottom

bpo-11913: Add README.rst to the distutils standard READMEs #563

Merged
merged 1 commit into from Apr 14, 2017
Merged

bpo-11913: Add README.rst to the distutils standard READMEs #563

merged 1 commit into from Apr 14, 2017

Conversation

@refi64
Copy link
Contributor

@refi64 refi64 commented Mar 8, 2017

No description provided.

@mention-bot
Copy link

@mention-bot mention-bot commented Mar 8, 2017

@kirbyfan64, thanks for your PR! By analyzing the history of the files in this pull request, we identified @merwok, @collinw, @jaraco, @birkenfeld and @freddrake to be potential reviewers.
@nedbat
Copy link
Member

@nedbat nedbat commented Mar 8, 2017

Does PyPI display README.md yet? If not, let's not pretend it's a standard format.
@dstufft
Copy link
Member

@dstufft dstufft commented Mar 8, 2017

@nedbat No.
@freddrake
Copy link
Member

@freddrake freddrake commented Mar 8, 2017

I agree with @nedbat about leaving out README.md; otherwise this looks good.

I don't see an issue on bpo for this; am I missing something?
@merwok
Copy link
Member

@merwok merwok commented Mar 8, 2017

There are efforts on the Warehouse side (discussed on various PyPA bug trackers and mailing lists) to support markdown; let’s not change the client docs before the server* actually supports it.

  • and possibly other server implementations like devpi and mirrors
@merwok
merwok requested changes Mar 8, 2017
View changes
Copy link
Member

@merwok merwok left a comment

Proposed changes don’t match current facts.
@refi64 refi64 force-pushed the refi64:distutils-readme branch from 291a93e to 24915dd Mar 8, 2017
@refi64
Copy link
Contributor Author

@refi64 refi64 commented Mar 8, 2017

@nedbat @freddrake @merwok Fixed.
@refi64 refi64 changed the title Add README.md and README.rst to the distutils standard READMEs Add README.rst to the distutils standard READMEs Mar 8, 2017
@freddrake
Copy link
Member

@freddrake freddrake commented Mar 8, 2017

LGTM, but @merwok should weigh in.
@merwok
merwok reviewed Mar 8, 2017
View changes
Copy link
Member

@merwok merwok left a comment

Looks good to me!
@vstinner
vstinner reviewed Mar 9, 2017
View changes
Lib/distutils/command/sdist.py Outdated
@@ -96,7 +96,7 @@ def checking_metadata(self):

sub_commands = [('check', checking_metadata)]

READMES = 'README', 'README.txt'
READMES = 'README', 'README.txt', 'README.rst'

This comment has been minimized.

Sign in to view
@vstinner

vstinner Mar 9, 2017
Member

nitpick: I prefer an explicit tuple: ('README', 'README.txt', 'README.rst')
@refi64 refi64 force-pushed the refi64:distutils-readme branch from 24915dd to 34a627f Mar 9, 2017
@refi64
Copy link
Contributor Author

@refi64 refi64 commented Mar 9, 2017

@Haypo Done.
@vstinner
vstinner requested changes Mar 9, 2017
View changes
Copy link
Member

@vstinner vstinner left a comment

Since it's a new feature, it should be documented in Misc/NEWS, maybe also in Doc/whatsnew/3.7.rst as well.
Doc/distutils/sourcedist.rst
called your setup script), and :file:`setup.cfg`
* Any of the standard README files (:file:`README`, :file:`README.txt`,
or :file:`README.rst`), :file:`setup.py` (or whatever you called your setup
script), and :file:`setup.cfg`.

This comment has been minimized.

Sign in to view
@vstinner

vstinner Mar 9, 2017
Member

You should document the change using a ".. versionchanged:: 3.7 xx" markup, see a few lines below.

This comment has been minimized.

Sign in to view
@ned-deily

ned-deily Mar 9, 2017
Member

Also, there should be an issue for this on bugs.python.org and the PR title should reference that issue. It looks like this is a duplicate of bpo-11913 which was originally closed as "wont fix". It could be reopened and used to track this.
@refi64 refi64 changed the title Add README.rst to the distutils standard READMEs bpo-11913: Add README.rst to the distutils standard READMEs Mar 27, 2017
@refi64 refi64 force-pushed the refi64:distutils-readme branch from 34a627f to 85ef3b4 Mar 27, 2017
@refi64
Copy link
Contributor Author

@refi64 refi64 commented Mar 27, 2017

@Haypo Done. (I would personally think this isn't worthy of going in whatsnew/3.7.rst, but if you still feel so, which section would it go under, and how do you reference PR's like :issue: does?)
@merwok
Copy link
Member

@merwok merwok commented Mar 27, 2017

This is a user-visible usability improvement, so it deserves a mention in whatsnew. I believe the section would be «improved modules». I don’t think there is markup to reference PRs, but you referencing the bpo bug is all that’s needed.
@vstinner
vstinner approved these changes Mar 27, 2017
View changes
@vstinner
Copy link
Member

@vstinner vstinner commented Mar 27, 2017

Please fix the conflict on Misc/NEWS (rebase your change).
@vstinner
Copy link
Member

@vstinner vstinner commented Mar 27, 2017

@kirbyfan64: Oh, and see also latest @merwok comment.
@refi64 refi64 force-pushed the refi64:distutils-readme branch from 85ef3b4 to e53d078 Mar 28, 2017
@refi64
Copy link
Contributor Author

@refi64 refi64 commented Mar 28, 2017

@Haypo Done.
@vstinner
Copy link
Member

@vstinner vstinner commented Mar 28, 2017

I fixed a conflict in whatsnew/3.7.rst
@merwok
merwok approved these changes Mar 28, 2017
View changes
@refi64 refi64 force-pushed the refi64:distutils-readme branch from fa7b9c2 to ef293bd Mar 30, 2017
@refi64
Copy link
Contributor Author

@refi64 refi64 commented Mar 30, 2017

Rebased again onto the latest news and whatsnew.
@refi64
bpo-11913: Add README.rst to the distutils standard READMEs list
cc6ddb9
@refi64 refi64 force-pushed the refi64:distutils-readme branch from ef293bd to cc6ddb9 Apr 14, 2017
@refi64
Copy link
Contributor Author

@refi64 refi64 commented Apr 14, 2017

Rebased...again...
@vstinner vstinner merged commit f9f87f0 into python:master Apr 14, 2017
4 checks passed
4 checks passed
@the-knights-who-say-ni
bedevere/issue-number Issue number 11913 found.
Details
@codecov
codecov/patch 100% of diff hit (target 100%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner
Copy link
Member

@vstinner vstinner commented Apr 14, 2017

I merged your enhancement. Thanks. I didn't notice that README.rst wasn't included by distutils. Maybe because it's included in setuptools, and I now always use setuptools?

Rebased again onto the latest news and whatsnew.

Rebased...again...

I'm sorry about that, but we are working on a solution for this very annoying file (Misc/NEWS):
python/core-workflow#66
zhangyangyu added a commit to zhangyangyu/cpython that referenced this pull request May 8, 2017
Ryan Gonzalez @zhangyangyu
bpo-11913: Add README.rst to the distutils standard READMEs list (pyt…
2305154 
…hon#563)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ned-deily ned-deily

@vstinner vstinner

@merwok merwok

Assignees
No one assigned
Labels
CLA signed type-enhancement
Projects
None yet
Milestone
No milestone
10 participants
@refi64 @mention-bot @nedbat @dstufft @freddrake @merwok @vstinner @ned-deily @brettcannon @the-knights-who-say-ni