satis.json schema #271

JamesRezo · 2015-11-25T08:55:22Z

This is a proposal for a satis file validation schema.
It can be tested with https://github.com/justinrainbow/json-schema :

validate-json /path/to/satis.json res/satis-schema.json

stof · 2015-11-25T09:59:17Z

res/satis-schema.json

+        "minimum-stability": {
+            "type": ["string"],
+            "description": "The minimum stability the packages must have to be install-able. Possible values are: dev, alpha, beta, RC, stable.",
+            "pattern": "^dev|alpha|beta|rc|RC|stable$"


using enum would be simpler

alcohol · 2015-11-25T10:00:22Z

It would probably be a good idea to validate the schema on every run. Not necessarily fail the run, but at least print out errors/warnings to notify the user.

stof · 2015-11-25T10:04:05Z

res/satis-schema.json

+        "require": {
+            "type": "object",
+            "description": "This is a hash of package name (keys) and version constraints (values) that are required to run this package.",
+            "additionalProperties": true


rather than true, this should use a schema object here, to force all additional properties to contain strings

stof · 2015-11-25T10:07:42Z

You are missing the config property (which is the same than the property of the composer package schema)

JamesRezo · 2015-11-25T10:41:13Z

@stof, i completely agree about require, that could be the same for repositories. But it's to Composer to support sub-schema I guess. Is it possible to have more than one validation file in Composer ?
I can of course add a config property and maybe something about authentication... but again, it's about to Composer to support such sub-schemas.
Also, minimum-stability is a copy of the Composer property.

What I mean is that it would be preferable Composer manages its definitions and then Satis could use it. How do you feel about this ?

I have no idea how to set abandoned specification here. It has to match

"abandoned" : {
    "vendor/name": true,
    "vendor/othername": "othervendor/othername",
   ...
}

It's clearly an object, but how to specify its properties ?

@alcohol. Yep, it's the idea I have in mind. But for now, il would be a lot a duplicate code between Composer and Satis.

stof · 2015-11-25T11:23:07Z

@JamesRezo require values are just strings, same for abandonned (well, string or boolean in this case).

stof · 2015-11-25T11:23:36Z

I can of course add a config property

if you don't add it at all, you forbid defining it in the satis config, which is wrong.

JamesRezo · 2015-11-25T14:35:54Z

Sorry for the name mismatch :-)

stof · 2015-11-25T16:39:52Z

res/satis-schema.json

+                }
+            }
+        },
+        "extra": {


extra does not make sense in the Satis config

alcohol · 2015-11-27T08:50:49Z

/cc @Seldaek got any feedback regarding this schema?

alcohol · 2015-11-27T08:52:30Z

src/Composer/Satis/Command/BuildCommand.php

+            if (!$skipErrors) {
+                throw $e;
+            }
+            $output->writeln(sprintf("<error>Skipping Exception '%s'.</error>", $e->getMessage()));


Better to use warning instead of error I think, since $skipErrors basically downgrades them. Don't need the pre-text either then, just the exception. E.g. sprintf('<warning>%s: %s</warning>', get_class($e), $e->getMessage()).

JamesRezo · 2015-12-15T07:43:24Z

Is there something blocking yet ?

satis.json schema

alcohol · 2015-12-15T08:34:52Z

No, I'll merge it, and if anything pops up I expect a new PR :-)

glensc · 2016-01-07T14:03:53Z

looks like this change made "homepage" key required, and it was not easy to figure out WHY the validation failed (the error is not self-explanatory). i had pretty much test each option to figure out what made validation to fail...

$ satis build libs.json  -vvv

  [Composer\Json\JsonValidationException]                       
  The json config file does not match the expected JSON schema  

Exception trace:
 () at /usr/share/php/Composer/Satis/Command/BuildCommand.php:269
 Composer\Satis\Command\BuildCommand->check() at /usr/share/php/Composer/Satis/Command/BuildCommand.php:128
 Composer\Satis\Command\BuildCommand->execute() at /usr/share/php/Symfony/Component/Console/Command/Command.php:256
 Symfony\Component\Console\Command\Command->run() at /usr/share/php/Symfony/Component/Console/Application.php:838
 Symfony\Component\Console\Application->doRunCommand() at /usr/share/php/Symfony/Component/Console/Application.php:189
 Symfony\Component\Console\Application->doRun() at /usr/share/php/Composer/Satis/Console/Application.php:52
 Composer\Satis\Console\Application->doRun() at /usr/share/php/Symfony/Component/Console/Application.php:120
 Symfony\Component\Console\Application->run() at /usr/bin/satis:13


build [--repository-url [REPOSITORY-URL]] [--no-html-output] [--skip-errors] [--] [<file>] [<output-dir>] [<packages>]...
$ cat libs.json
{
        "name": "libs",
        "description": "Composer Libraries Repository",
        "require-all": true,
        "repositories": [
        ]
}

glensc · 2016-01-07T14:16:26Z

also, "name" seems to be required now, i had repo without name working fine before (generated without html output), and even now the name seems to be optional

so i'd send PR to make "name" and "homepage" optional again, ok?

stof · 2016-01-07T14:17:38Z

Well, the BuildCommand should display validation errors properly (the JsonValidationException contains some details)

satis.json schema

1177684

stof reviewed Nov 25, 2015
View reviewed changes

Following @Stog suggestions

dc3c830

stof reviewed Nov 25, 2015
View reviewed changes

res/satis-schema.json

}

}

},

"extra": {

Copy link

Contributor

stof Nov 25, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra does not make sense in the Satis config

JamesRezo added 3 commits November 25, 2015 17:52

Fix typos, @stof suggestions

873335a

validate the schema on every run

0351ffa

Check syntax before validating schema

32edf0b

alcohol reviewed Nov 27, 2015
View reviewed changes

JamesRezo added 3 commits November 27, 2015 10:14

warning instead of error, following @alcohol suggestion

a778f84

shorter text in output

7006f0f

add notify-batch in schema

8f538dd

alcohol added a commit that referenced this pull request Dec 15, 2015

Merge pull request #271 from JamesRezo/feature/schema

c568983

satis.json schema

alcohol merged commit c568983 into composer:master Dec 15, 2015

JamesRezo deleted the feature/schema branch December 15, 2015 09:04

glensc mentioned this pull request Jan 7, 2016

include error details about validation errors #293

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

satis.json schema #271

satis.json schema #271

JamesRezo commented Nov 25, 2015

stof Nov 25, 2015

alcohol commented Nov 25, 2015

stof Nov 25, 2015

stof commented Nov 25, 2015

JamesRezo commented Nov 25, 2015

stof commented Nov 25, 2015

stof commented Nov 25, 2015

JamesRezo commented Nov 25, 2015

stof Nov 25, 2015

alcohol commented Nov 27, 2015

alcohol Nov 27, 2015

JamesRezo commented Dec 15, 2015

alcohol commented Dec 15, 2015

glensc commented Jan 7, 2016

glensc commented Jan 7, 2016

stof commented Jan 7, 2016

satis.json schema #271

satis.json schema #271

Conversation

JamesRezo commented Nov 25, 2015

stof Nov 25, 2015

Choose a reason for hiding this comment

alcohol commented Nov 25, 2015

stof Nov 25, 2015

Choose a reason for hiding this comment

stof commented Nov 25, 2015

JamesRezo commented Nov 25, 2015

stof commented Nov 25, 2015

stof commented Nov 25, 2015

JamesRezo commented Nov 25, 2015

stof Nov 25, 2015

Choose a reason for hiding this comment

alcohol commented Nov 27, 2015

alcohol Nov 27, 2015

Choose a reason for hiding this comment

JamesRezo commented Dec 15, 2015

alcohol commented Dec 15, 2015

glensc commented Jan 7, 2016

glensc commented Jan 7, 2016

stof commented Jan 7, 2016