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 command to generate json schema #2996

Merged
merged 2 commits into from
Aug 21, 2019

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).

src/JsonSchema/Command/JsonSchemaGenerateCommand.php Outdated Show resolved Hide resolved
src/JsonSchema/SchemaFactory.php Outdated Show resolved Hide resolved
tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php Outdated Show resolved Hide resolved
@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.

None yet

4 participants