More validation of packaging data #154
Conversation
|
This is taking shape. I have added an environment variable FLIT_ALLOW_INVALID=1 flit build |
|
@Carreau I've extended the PEP 440 checks on the version number. Now it tries to normalise the version to the canonical PEP 440 form, with an error if it can't do that, and a warning if it can do that and it has to change the version. |
takluyver
changed the title
| @@ -139,6 +139,19 @@ Environment variables | |||
| See :ref:`uploading packages with environment variables <upload_envvars>` | |||
| for more information. | |||
|
|
|||
| .. envvar:: FLIT_ALLOW_INVALID | |||
There was a problem hiding this comment.
FLIT_ALLOW_INVALID_METADATA ? You may want invalid other things later on ?
There was a problem hiding this comment.
I think I'll live with that risk. I'm not sure that the scope of 'metadata' is very clear, e.g. entry points are not part of the [tool.flit.metadata] table.
This is meant to be an escape hatch for when a check is wrong, so I don't think it needs to be fine-grained.
There was a problem hiding this comment.
I'm ok with it, it was mostly a question. You're the boss, boss.
There was a problem hiding this comment.
It was a point worth thinking about :-)
flit/validate.py
Outdated
| # Linux, Unix, AIX, etc. | ||
| # use ~/.cache if empty OR not set | ||
| xdg = os.environ.get("XDG_CACHE_HOME", None) or ( | ||
| os.path.expanduser('~/.cache')) |
There was a problem hiding this comment.
Hard to read indentation (IMHO)
There was a problem hiding this comment.
Is the new commit better? I'm not quite sure what was making it tricky.
There was a problem hiding this comment.
It's just no indentation after the or (\n, looks to my eyes (brain?), like a missing assignment. At least I don't easily see the it's part of the above assignment. It may be britany's brain flakes are of low quality.
There was a problem hiding this comment.
Ah, I see it now. It was so misleading I didn't even notice what it was meant to be ;-)
flit/validate.py
Outdated
| log.info('Fetching list of valid trove classifiers') | ||
| resp = requests.get( | ||
| 'https://pypi.python.org/pypi?%3Aaction=list_classifiers') | ||
| resp.raise_for_status() |
There was a problem hiding this comment.
possibly find the corresponding URL for warehouse, and use as a fallback ?
There was a problem hiding this comment.
I've found https://pypi.org/pypi?%3Aaction=list_classifiers , but I'm asking whether it's something we can rely on before I switch to it: pypi/warehouse#1300
There was a problem hiding this comment.
Donald says it's safe to rely on, so I've switched to that.
|
Thanks for the review :-) . Do you want to look any more at it, or shall one of us press the big green button? |
|
I'll give another read in a few hours if you wish, but otherwise, let's
test it !
…On Dec 12, 2017 12:45, "Thomas Kluyver" ***@***.***> wrote:
Thanks for the review :-) . Do you want to look any more at it, or shall
one of us press the big green button?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#154 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAUez9IyG3-JRMb4K6WU2Wpgis2Z_TsGks5s_mdUgaJpZM4Q7NuR>
.
|
|
I've just tried it with a bunch of projects I've got on this machine, and it wasn't causing any problems, so I'm going to merge it now. I'll give it a day or two in master before making a new release. |
This should make it easier to catch mistakes in packaging sooner.
The design is slightly awkward: a series of validation functions build up a list of problems. These are logged at 'error' level, and then a ConfigError is raised. I'm doing it this way so that it's easy to see multiple problems at once, not just the first one it finds.