KSP-CKAN / CKAN

The Comprehensive Kerbal Archive Network
https://forum.kerbalspaceprogram.com/index.php?/topic/197082-*
Other
1.98k stars 347 forks source link

NetKAN needs better pull-request validation #346

Closed pjf closed 9 years ago

pjf commented 9 years ago

Right now the NetKAN repo is just doing JSON validation.

Ideally, we should have it try to inflate changed files and validate the results.

Alternatively (and probably nicer to travis) we should have a script that does this:

That could be flipped on for travis, but would also streamline the basic sanity check process.

pjf commented 9 years ago

I think what we really need here is a netkan --no-download switch, which simply grabs the metadata and makes sure everything looks good. Travis can work with that just fine, we can use it to output real .ckan files, and then the heavy-duty verifier can check those.

This won't spot things like booched file paths into the zip, but it will catch most of the most common errors.

pjf commented 9 years ago

Better still, we just implement proper license validation in ckan.dll, which means that netkan will throw an error automatically. This catches the most common case where we inflate a document with a machine-unfriendly license string.

pjf commented 9 years ago

So, I've added and are about to merge tests for the following:

Because our tests don't actually do any netkan inflation, the second can report a missing license when we'd inflate it just fine (eg: when the KerbalStuff license is a valid license string in our spec). In those cases, one can set "x_netkan_license_ok" : true to disable the check.

Heads up @Ippo343 on this, as the tests will go live shortly. :)