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

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

Merged
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

@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 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 commented Mar 8, 2017

@nedbat No.

@brettcannon brettcannon added the type-feature A feature request or enhancement label Mar 8, 2017
@freddrake
Copy link
Member

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 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

Copy link
Member

@merwok merwok left a comment

Choose a reason for hiding this comment

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

Proposed changes don’t match current facts.

@refi64
Copy link
Contributor Author

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

LGTM, but @merwok should weigh in.

Copy link
Member

@merwok merwok left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@@ -96,7 +96,7 @@ def checking_metadata(self):

sub_commands = [('check', checking_metadata)]

READMES = 'README', 'README.txt'
READMES = 'README', 'README.txt', 'README.rst'
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: I prefer an explicit tuple: ('README', 'README.txt', 'README.rst')

@refi64
Copy link
Contributor Author

refi64 commented Mar 9, 2017

@Haypo Done.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Since it's a new feature, it should be documented in Misc/NEWS, maybe also in Doc/whatsnew/3.7.rst as well.

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`.
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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
Copy link
Contributor Author

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 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
Copy link
Member

Please fix the conflict on Misc/NEWS (rebase your change).

@vstinner
Copy link
Member

@kirbyfan64: Oh, and see also latest @merwok comment.

@refi64
Copy link
Contributor Author

refi64 commented Mar 28, 2017

@Haypo Done.

@vstinner
Copy link
Member

I fixed a conflict in whatsnew/3.7.rst

@refi64
Copy link
Contributor Author

refi64 commented Mar 30, 2017

Rebased again onto the latest news and whatsnew.

@refi64
Copy link
Contributor Author

refi64 commented Apr 14, 2017

Rebased...again...

@vstinner vstinner merged commit f9f87f0 into python:master Apr 14, 2017
@vstinner
Copy link
Member

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

jaraco pushed a commit that referenced this pull request Dec 2, 2022
Bumps [redis](https://github.com/redis/redis-py) from 4.3.3 to 4.3.4.
- [Release notes](https://github.com/redis/redis-py/releases)
- [Changelog](https://github.com/redis/redis-py/blob/master/CHANGES)
- [Commits](redis/redis-py@v4.3.3...v4.3.4)

---
updated-dependencies:
- dependency-name: redis
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants