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

Improve README #1114

Merged
merged 9 commits into from
Jan 2, 2019
Merged

Improve README #1114

merged 9 commits into from
Jan 2, 2019

Conversation

obestwalter
Copy link
Member

@obestwalter obestwalter commented Jan 2, 2019

restart with a fresh addressing suggestions from previous PR

re-opening. Based on botched up #1079

see #819

preview: https://github.com/tox-dev/tox/tree/819-improve-readme

@obestwalter obestwalter changed the title restart with a fresh addressing suggestions from previous PR Improve README Jan 2, 2019
@obestwalter
Copy link
Member Author

Looks like Github "sanitizes" all size hints: github/markup#295

That would be one argument for switching to markdown and simply using HTML for things like that. I'd really like the logo right aligned and smaller next to the First heading and with markdown that worked.

Alternative in rst would be to keep a scaled copy and use that here.

@gaborbernat
Copy link
Member

I'm fine with switching to markdown, as long as works with both pypi and github 👍

@obestwalter
Copy link
Member Author

size is sanitized but you can use the raw directive - which is obviously much safer 🤣

@gaborbernat
Copy link
Member

Do you think raw will work on PyPi too?

@obestwalter
Copy link
Member Author

If there are no glaring errors left, I think we can merge this now.

@obestwalter
Copy link
Member Author

Do you think raw will work on PyPi too?

arg ... I don't know. Shall we try to find outor just switch to markdown? Thanks to pandoc it's no pain either way to switch formats.

@obestwalter
Copy link
Member Author

obestwalter commented Jan 2, 2019

A quick search suggests that it doesn't: https://stackoverflow.com/a/49107899/2626627

Question is if PyPI supports pure HTML coming from a markdown document either - I'll try it out on the test instance.

Stack Overflow
I have a README.rst in my project hosted on GitHub. It renders fine on GitHub but fails to render on PyPi i.e. I see raw content on the PyPi project description page.

I have looked through similar

@obestwalter
Copy link
Member Author

obestwalter commented Jan 2, 2019

raw directive is not supported for rst (readme-renderer also flags that).

$ twine check dist/*

[...]
The project's long_description has invalid markup which will not be rendered on PyPI. The following syntax errors were detected:
line 4: Warning: "raw" directive disabled.
Checking distribution dist/tox-0.9.1.tar.gz: Failed
The project's long_description has invalid markup which will not be rendered on PyPI. The following syntax errors were detected:
line 4: Warning: "raw" directive disabled.
line 4: Warning: "raw" directive disabled.

@obestwalter
Copy link
Member Author

obestwalter commented Jan 2, 2019

It does work with markdown though 🍾

scrot

$ twine check dist/*

Checking distribution dist/tox-0.9.2-py2.py3-none-any.whl: Passed
Checking distribution dist/tox-0.9.2.tar.gz: Passed

@obestwalter
Copy link
Member Author

o.k. I think markdown it is then and I'll update the release / check bit also now.

@gaborbernat
Copy link
Member

The CI still fails... 🤔

@obestwalter
Copy link
Member Author

Are we not checking the README with readme-renderer anymore? Can't find it, but might be packaged differently now.

@gaborbernat
Copy link
Member

gaborbernat commented Jan 2, 2019

readme-renderer is now part of twine 👍 because of PEP-517. see pypa/twine#395

in the meantime just add it to the package readme tox deps section alongside twine the md extras

@obestwalter
Copy link
Member Author

2019-01-02T14:28:36.5844635Z package_description runtests: commands[1] | twine check '/home/vsts/work/1/s/.tox/package_description/tmp/build/*'
2019-01-02T14:28:37.0299156Z /home/vsts/work/1/s/.tox/package_description/lib/python3.7/site-packages/readme_renderer/markdown.py:38: UserWarning: Markdown renderers are not available. Install 'readme_render[md]' to enable Markdown rendering.
2019-01-02T14:28:37.0299940Z   warnings.warn(_EXTRA_WARNING)
2019-01-02T14:28:37.0300451Z Checking distribution /home/vsts/work/1/s/.tox/package_description/tmp/build/tox-3.6.2.dev13+g4e56ae3-py2.py3-none-any.whl: Failed
2019-01-02T14:28:37.0300991Z The project's long_description has invalid markup which will not be rendered on PyPI. The following syntax errors were detected:
2019-01-02T14:28:37.0301214Z line 4: Warning: "raw" directive disabled.

It's happening, but I haven't figured out yet where. Haven't looked at how we do stuff now yet, so will check.readme-renderer need top be installed with [md] for it to work.

@obestwalter
Copy link
Member Author

readme-renderer is now part of twine +1 because of PEP-517.

cool but maybe without markdown check? I tried locally and still needed it pip install the md stuff.

@obestwalter
Copy link
Member Author

yep, that's the reason. Will check if there is an issue about this already. I guess we are not the only ones who want to do this and run into it.

@gaborbernat
Copy link
Member

gaborbernat commented Jan 2, 2019

https://github.com/pypa/twine/blob/master/setup.py#L84 could be a twine bug 👍 they probably should forward the extras 👍

GitHub
Utilities for interacting with PyPI. Contribute to pypa/twine development by creating an account on GitHub.

@obestwalter
Copy link
Member Author

obestwalter commented Jan 2, 2019

I just read a bit about it and am a bit confused about the state of markdown in readme-renderer, as apparently it can never fail, so this could mean its never checked but only fails when md extras are not installed. I'll add a dep in our tox env to install it extra but will get in touch with PyPA about how to solve this generally.

pypa/twine#421

@todo
Copy link

todo bot commented Jan 2, 2019

installing readme-renderer[md] should not be necessary

tox/tox.ini

Lines 44 to 48 in 7f88569

# TODO installing readme-renderer[md] should not be necessary
readme-renderer[md] >= 24.0
pip >= 18.0.0
skip_install = true
extras =


This comment was generated by todo based on a TODO comment in 7f88569 in #1114. cc @tox-dev.

@obestwalter
Copy link
Member Author

obestwalter commented Jan 2, 2019

I asked about it on the list. If there is consensus about making markdown support the default it could be "fixed" in readme-renderer diretly by adding the dependency to the normal install_requires deps and everything else can go away: https://groups.google.com/forum/#!topic/pypa-dev/BevObZsKVOQ

Google Groups allows you to create and participate in online forums and email-based groups with a rich experience for community conversations.

@gaborbernat gaborbernat merged commit f448692 into master Jan 2, 2019
@delete-merged-branch delete-merged-branch bot deleted the 819-improve-readme branch January 2, 2019 16:07
@obestwalter obestwalter added this to the 3.7 milestone Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants