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 way to turn twine check warnings into errors #562

Closed
Julian opened this issue Jan 24, 2020 · 12 comments · Fixed by #715
Closed

Add a way to turn twine check warnings into errors #562

Julian opened this issue Jan 24, 2020 · 12 comments · Fixed by #715
Labels
feature request question Discussion/decision needed from maintainers

Comments

@Julian
Copy link
Contributor

Julian commented Jan 24, 2020

Hi!

twine check looks like beyond just failing if the output is invalid, it can output some warnings. E.g.:

readme run-test: commands[1] | /Users/julian/Development/jsonschema/.tox/readme/bin/python -m twine check '/Users/julian/Development/jsonschema/.tox/readme/tmp/dist/*'
Checking /Users/julian/Development/jsonschema/.tox/readme/tmp/dist/jsonschema-3.2.1.dev35+g9131e5cd.d20200124-py2.py3-none-any.whl: PASSED, with warnings
  warning: `long_description_content_type` missing.  defaulting to `text/x-rst`.
Checking /Users/julian/Development/jsonschema/.tox/readme/tmp/dist/jsonschema-3.2.1.dev35+g9131e5cd.d20200124.tar.gz: PASSED, with warnings
  warning: `long_description_content_type` missing.  defaulting to `text/x-rst`.

Not sure if there are other warnings it raises today -- twine check looks like it exits non-zero if an actual error occurs, but for nitpicky's sake, it seems like it'd be nice to be able to fail for warnings too.

(Apologies if there's some other way besides a CLI argument to do this and I missed it)

@di
Copy link
Sponsor Member

di commented Jan 24, 2020

Sorry, it's not currently possible to turn these into errors.

In addition: these are warnings and not errors for a reason: twine check ensures that the distribution won't fail on upload to PyPI. The distribution is still valid, here, so twine check should pass without errors.

Can you give a bit more detail about why you want this to cause a failure?

@Julian
Copy link
Contributor Author

Julian commented Jan 24, 2020

The warnings seem like they're there to warn :) -- if there's literally nothing bad about them all good, but by having them it sounds like there's something to fix -- when twine check is run by a human the idea is presumably that you'd see them in the console, but when twine check isn't (e.g. in the above, where it runs in CI), no one would ever notice them to decide whether to fix or not (unless they fail the build, hence this kind of option). So if a warning was used to say, e.g., "some time in the future PyPI won't like this", you're (I'm) not going to notice until it actually breaks.

For analogy, e.g. sphinx-build takes a -W which turns warnings into errors (I assume for the same reasoning, though sphinx warnings are often borderline errors anyhow).

Does that make any more sense?

@bhrutledge
Copy link
Contributor

That sounds reasonable to me; I'd probably do something like twine check --strict. @pypa/twine-maintainers what do y'all think?

@bhrutledge bhrutledge added question Discussion/decision needed from maintainers feature request and removed enhancement labels Feb 1, 2020
@sigmavirus24
Copy link
Member

I disagree @bhrutledge. Those warnings in particular are informational and don't require fixing. They're useful if and only if you're using markdown for the long description. Unlike sphinx the (I think only) warnings here are not guaranteed to be error worthy in what I'd guess is the majority use-case.

@Julian
Copy link
Contributor Author

Julian commented Feb 1, 2020

Cool, that's fine with me too if that's the case -- that means twine will guarantee that warnings emitted by it will never require action?

@di
Copy link
Sponsor Member

di commented Feb 1, 2020

I think we can probably make that guarantee.

@Julian
Copy link
Contributor Author

Julian commented Feb 1, 2020

Sounds good thanks!

@Julian Julian closed this as completed Feb 1, 2020
@ssbarnea
Copy link

Can we please reopen this? It makes no sense to not have a strict mode. I recently uploaded a package with empty description on pypi because during conversion from README.rst to README.md I forgot to update the setup.cfg file.

Yes, I did had a twine check as part of the release process, but missing a warning on the console is a very easy mistake to make.

@gaborbernat
Copy link

IMHO it's sane to have --strict here; while PyPi will accept the upload it's probably worth fixing these warnings if you care that your package looks good on PyPi.

@bhrutledge
Copy link
Contributor

As the maintainer with the narrowest perspective, twine check --strict makes sense to me as a way to make uploading to PyPI friendlier. That said, I'm going to defer to the @pypa/twine-maintainers with a broader perspective.

@Julian
Copy link
Contributor Author

Julian commented Apr 18, 2020

(Re-opening, since I agree that that example seems to already not meet the "guarantee" above, but yeah obviously if there's still disagreement I won't feel bad if this is re-closed)

@bhrutledge
Copy link
Contributor

twine check --strict has been released: https://pypi.org/project/twine/3.3.0/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request question Discussion/decision needed from maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants