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

Migrated README to Markdown (PEP 566) #66

Merged
merged 3 commits into from Apr 2, 2018

Conversation

Projects
None yet
2 participants
@9999years
Copy link
Contributor

9999years commented Mar 17, 2018

Migrated README.rst to README.md and updated setup.py to match;
this is in line with PEP 566 and the new long_description_content_type
arg in setuptools.

Also updated MANIFEST.in to include the new README.md with include *.md as suggested by check-manifest — is that the right way to do that?

See also:

Migrated README to Markdown (PEP 566)
Migrated `README.rst` to `README.md` and updated `setup.py` to match;
this is in line with PEP 566 and the new long_description_content_type
arg in setuptools.

Also updated `MANIFEST.in` to include the new `README.md`.

See:

 * [Relevant section in PEP 566](https://www.python.org/dev/peps/pep-0566/#description-content-type-optional)
 * [The CommonMark specification](http://spec.commonmark.org)
 * [The Description-Content-Type field documentation in the Python
   packaging documentation](https://packaging.python.org/specifications/core-metadata/#description-content-type-optional)

@9999years 9999years force-pushed the 9999years:master branch from 825f25a to 5be0970 Mar 18, 2018

setup.py Outdated
#
# This field corresponds to the "Description-Content-Type" metadata field:
# https://packaging.python.org/specifications/core-metadata/#description-content-type-optional
long_description_content_type='text/markdown', # Optional

This comment has been minimized.

Copy link
@di

di Mar 19, 2018

Member

Might want to add a note here that while this field is technically optional, it's required if you want a Markdown long_description.

@9999years

This comment has been minimized.

Copy link
Contributor Author

9999years commented Mar 20, 2018

Added a comment to clarify:

Optional if long_description is written in RST but required for plain-text or Markdown; if unspecified, "applications should attempt to render [the long_description] as text/x-rst; charset=UTF-8 and fall back to text/plain if it is not valid rst" (link)

setup.py Outdated
# plain-text or Markdown; if unspecified, "applications should attempt to
# render [the long_description] as text/x-rst; charset=UTF-8 and fall back
# to text/plain if it is not valid rst" (see link below)
#

This comment has been minimized.

Copy link
@di

di Mar 20, 2018

Member

I think this is in the wrong place, it should be below long_description_content_type=....

This comment has been minimized.

Copy link
@9999years

9999years Mar 20, 2018

Author Contributor

Yeah, noticed just after pushing. Should be updated now.

Clarified when content_type is optional
Added a comment clarifying that `long_description_content_type` in `setup()`
is only optional if the `long_description` is written in RST and a quote
from relevant documentation.

@9999years 9999years force-pushed the 9999years:master branch from 194a085 to 10bef87 Mar 20, 2018

@di
Copy link
Member

di left a comment

Some small changes, otherwise this LGTM.

README.md Outdated
# A sample Python project

A sample project that exists as an aid to the [Python Packaging User
Guide][packaging guide]’s [Tutorial on Packaging and Distributing

This comment has been minimized.

Copy link
@di

di Mar 20, 2018

Member

Could you replace the character with ' here?

README.md Outdated
Guide][packaging guide]’s [Tutorial on Packaging and Distributing
Projects][distribution tutorial].

This projects does not aim to cover best practices for Python project

This comment has been minimized.

Copy link
@di

di Mar 20, 2018

Member

There's a typo here that's not your fault, should be "this project".

setup.py Outdated
# Denotes that our long_description is in Markdown; valid values are
# text/plain, text/x-rst, and text/markdown
#
# Optional if long_description is written in rst but required for

This comment has been minimized.

Copy link
@di

di Mar 20, 2018

Member

Could you change rst here to reStructuredText? Since newcomers might not know what "rst" stands for.

setup.py Outdated
#
# This field corresponds to the "Description-Content-Type" metadata field:
# https://packaging.python.org/specifications/core-metadata/#description-content-type-optional
long_description_content_type='text/markdown', # Optional

This comment has been minimized.

Copy link
@di

di Mar 20, 2018

Member

Maybe change # Optional here to # Optional (see note above)?

Typos and clarifications
* Changed true quotes to ASCII quotes in `README.md`
* Fixed a typo (`This projects` instead of `This project`) in
  `README.md`
* Changed `rst` to `reStructuredText (rst)` in `setup.py` for
  people who might not be familiar with the format
* Added small note to `# Optional` comment on the
  `long_description_content_type` arg in `setup.py:L70`
@9999years

This comment has been minimized.

Copy link
Contributor Author

9999years commented Mar 20, 2018

OK, fixed.

@di

di approved these changes Mar 20, 2018

Copy link
Member

di left a comment

Great! I'm going to hold off on merging this until pypa/warehouse#869 is closed.

@9999years

This comment has been minimized.

Copy link
Contributor Author

9999years commented Apr 2, 2018

Can we get this merged now that pypa/warehouse#869 is closed?
Thanks!

@di di merged commit c64cc7d into pypa:master Apr 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@di

This comment has been minimized.

Copy link
Member

di commented Apr 2, 2018

Thanks @9999years!

@ocefpaf ocefpaf referenced this pull request May 11, 2018

Merged

no setuptools-markdown #68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.