Skip to content

Comments

add command to generate json schema#2996

Merged
dunglas merged 2 commits intoapi-platform:masterfrom
jockos:add-json-schema-generate-command
Aug 21, 2019
Merged

add command to generate json schema#2996
dunglas merged 2 commits intoapi-platform:masterfrom
jockos:add-json-schema-generate-command

Conversation

@jockos
Copy link
Contributor

@jockos jockos commented Aug 19, 2019

Add a command to generate a JSON Schema for a resource.
Can be used for the whole resource or by operation

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Almost good! Just one little more change. The CI must also be green (PHP CS Fixer, PHPStan etc).

@jockos jockos force-pushed the add-json-schema-generate-command branch 2 times, most recently from 1736836 to 4b9405d Compare August 20, 2019 07:56
Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

When the CI will be green!

@jockos jockos force-pushed the add-json-schema-generate-command branch 7 times, most recently from 49bb11d to 672949c Compare August 20, 2019 20:50
->setName('api:json-schema:generate')
->setDescription('Generates the JSON Schema for a resource operation.')
->addArgument('resource', InputArgument::REQUIRED, 'The Fully Qualified Class Name (FQCN) of the resource')
->addOption('itemOperation', null, InputOption::VALUE_OPTIONAL, 'The item operation')
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the InputOption::VALUE_OPTIONAL should actually be InputOption::VALUE_REQUIRED.

From the documentation:

InputOption::VALUE_REQUIRED

This value is required (e.g. --iterations=5 or -i5), the option itself is still optional

Copy link
Contributor

Choose a reason for hiding this comment

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

Another thing, don't we usually use dashes in option names, not camelCase?

@jockos jockos force-pushed the add-json-schema-generate-command branch 3 times, most recently from e9436f2 to 91b530e Compare August 20, 2019 21:15
->addOption('itemOperation', null, InputOption::VALUE_REQUIRED, 'The item operation')
->addOption('collectionOperation', null, InputOption::VALUE_REQUIRED, 'The collection operation')
->addOption('format', null, InputOption::VALUE_REQUIRED, 'The response format', (string) $this->formats[0])
->addOption('type', null, InputOption::VALUE_REQUIRED, 'Use this option to set the type of output', 'input');
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps:

The type of schema to generate (input or output)

@jockos jockos force-pushed the add-json-schema-generate-command branch 3 times, most recently from 45d4e9d to 8aef8bf Compare August 21, 2019 05:49
@jockos jockos force-pushed the add-json-schema-generate-command branch from 8aef8bf to 6ff741c Compare August 21, 2019 05:52
@dunglas dunglas merged commit 7ac5857 into api-platform:master Aug 21, 2019
@dunglas
Copy link
Member

dunglas commented Aug 21, 2019

Thanks @jockos!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants