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

fix(metadata): no skolem iri by default #5046

Merged
merged 3 commits into from
Oct 7, 2022

Conversation

soyuka
Copy link
Member

@soyuka soyuka commented Oct 3, 2022

Q A
Branch? bug fix (bc break)
Tickets #5035
License MIT

On 3.0 this behavior will be the default one, I'll add it only in 3.0 so that one can disable the skolemIri generation. We break the compatibility here, so let's revert for 2.7.

@tucksaun
Copy link

tucksaun commented Oct 4, 2022

Thank you @soyuka for working on a fix for this issue 🙏

Though, regarding the "fix" for 3.0, may I suggest the following patch:
https://github.com/api-platform/core/compare/3.0...tucksaun:api-platform-core:fix-api-property-iris-anonymous-resources?expand=1

It should remove the need for patching ApiPlatform\Metadata\ApiProperty that's why I'm commenting here ;)
If the approach looks viable to you I can open the PR stat.

src/Metadata/ApiProperty.php Outdated Show resolved Hide resolved
@soyuka
Copy link
Member Author

soyuka commented Oct 7, 2022

@tucksaun thanks for that I added it to the fix here, iri is too confusing however so let's fix this properly by adding a new genid option to ApiProperty. It'll default to false in 2.7 and true in 3.0.

@soyuka soyuka force-pushed the fix/bc-skolem-iris branch 3 times, most recently from c2f6f99 to 05d3320 Compare October 7, 2022 08:27
@soyuka soyuka force-pushed the fix/bc-skolem-iris branch from 05d3320 to 68fe207 Compare October 7, 2022 08:28
@soyuka soyuka merged commit 0d8b516 into api-platform:2.7 Oct 7, 2022
@soyuka soyuka deleted the fix/bc-skolem-iris branch October 7, 2022 08:55
@tucksaun
Copy link

Finally got the chance to give it a go and it seems everything is working properly.

Thank you @soyuka!
And good luck with 3.0 stabilization 🤗

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.

2 participants