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

CXP-1300: Adds first product meta schema #17982

Merged
merged 3 commits into from
Sep 28, 2022
Merged

CXP-1300: Adds first product meta schema #17982

merged 3 commits into from
Sep 28, 2022

Conversation

tseho
Copy link
Contributor

@tseho tseho commented Sep 21, 2022

Why a new dependency ?

The current JSON Schema library we use in the PIM stopped updating the spec at the Draft-4. It does not support all the features we have in the meta schema.
jsonrainbow/json-schema#658 🎉

2 new alternatives emerged in the community, and after trying both, I went with opis/json-schema because the API is similar and it supports draft-2020-12, draft-2019-09, draft-07 and draft-06.

yes, we need an ADR.

Definition Of Done (for Core Developer only)

  • Tests
  • Migration & Installer
  • PM Validation (Story)
  • Changelog (maintenance bug fixes)
  • Tech Doc

@tseho tseho requested a review from a team as a code owner September 21, 2022 14:53
@masacc
Copy link
Contributor

masacc commented Sep 22, 2022

don't forget to link the API docs PR to fulfill the ticket

@@ -56,7 +56,7 @@ public function validSchemaDataProvider(): array
{
return [
'0.0.1 with valid schema' => [
'schema' => <<<'JSON'
'schema' => <<<'JSON_WRAP'
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain the diff pls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rector complained about it.
https://github.com/rectorphp/rector/blob/main/rules/Php73/Rector/String_/SensitiveHereNowDocRector.php

I'm not sure why this rule exists.
Either we don't care and uses _WRAP or we disable the rule.
WDYT ?

@tseho tseho merged commit 05831c8 into master Sep 28, 2022
@tseho tseho deleted the CXP-1300 branch September 28, 2022 15:17
Beninho pushed a commit that referenced this pull request Sep 30, 2022
* CXP-1300: Adds first product meta schema

* CXP-1300: remove  &  from schema

* CXP-1300: fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants