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

Add a CI lint task to check st2client's README.md #5191

Closed
blag opened this issue Mar 16, 2021 · 9 comments · Fixed by #5306
Closed

Add a CI lint task to check st2client's README.md #5191

blag opened this issue Mar 16, 2021 · 9 comments · Fixed by #5306

Comments

@blag
Copy link
Contributor

blag commented Mar 16, 2021

We need to make sure that the st2client README.rst file is acceptable to PyPI, since any syntax errors in it will cause the push_st2client task of the st2cd.st2_finalize_release workflow to fail.

We can check the syntax using the same renderer that PyPI itself uses:

# Use the same README renderer that PyPI uses to catch syntax issues in the
# README.rst file  # st2client uses README.rst
# https://pypi.org/help/#description-content-type
# https://pypi.org/project/readme-renderer
# https://packaging.python.org/tutorials/packaging-projects/#description
echo "Checking README.rst syntax"
virtualenv venv-st2client-readme-checker
. venv-st2client-readme-checker/bin/activate
pip install --upgrade readme_renderer
python -m readme_renderer README.rst
deactivate

It would be nice if we could catch these errors before release, which means that we should create a step in our CI tooling to check it before any bad changes get merged.

@Kami
Copy link
Member

Kami commented Mar 16, 2021

One option would also be to move it to .rst format (same as changelog) and just run rstcheck on it (we already run it on CHANGELOG.rst).

So perhaps not a bad thing from consistnecy perspective and you could argue rst is also more "native" for Python (our docs are also in rst, etc.) :)

@blag
Copy link
Contributor Author

blag commented Mar 16, 2021

We could do that, but the readme_renderer is the exact one that is used by PyPI to render READMEs, so I would prefer that one:

To check a description locally for validity, you may use readme_renderer, which is the same description renderer used by PyPI.

@Kami
Copy link
Member

Kami commented Mar 16, 2021

Yeah, that works.

And sorry, I was also talking about top level project README.md (and not just the client one) which currently uses Markdown.

@blag
Copy link
Contributor Author

blag commented Mar 16, 2021

Gotcha.

I think we originally intended to make the st2* directories all their own packages to the point that we could release them all individually on PyPI. If that was our intent, then we should include better descriptions than we currently do, and part of that can be copying the base README text. If we do that, then we should still at least use readme_renderer on it (but yep, we can/should also use rstcheck on RST READMEs).

@stale
Copy link

stale bot commented Jun 22, 2021

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

@stale stale bot added the stale label Jun 22, 2021
@amanda11
Copy link
Contributor

amanda11 commented Jun 28, 2021

At moment we have README.rst in place, but even running the commands suggested, I found that the readme renderer still passed.
However following found errors (which twine check and rstcheck did not report errors with)

python setup.py check -r -s
running check
warning: Check: This command has been deprecated. Use `twine check` instead: https://packaging.python.org/guides/making-a-pypi-friendly-readme#validating-restructuredtext-markup
warning: Check: The project's long_description has invalid markup which will not be rendered on PyPI. The following syntax errors were detected:
line 7: Warning: Cannot analyze code. No Pygments lexer found for "base".
error: Please correct your package.

@stale stale bot removed the stale label Jun 28, 2021
@amanda11 amanda11 added this to the 3.6.0 milestone Jun 28, 2021
@amanda11
Copy link
Contributor

I can start looking at this, as familiar given problem happened in middle of last release process.

@amanda11
Copy link
Contributor

Interestingly when add to Makefile, python3 setup.py check -r - accepts README.rst with sourcecode with unknown lexer, but python2 setup.py check -r -> will find error.

@amanda11
Copy link
Contributor

rst-lint however finds the error, so will use that as well as readme-renderer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants