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

feat: replace @type with type and roundTrip testing #114

Merged

Conversation

wolf4ood
Copy link
Contributor

@wolf4ood wolf4ood commented Dec 6, 2024

WHAT

This PR replaces the reference to @type with type in the example and the required field section.

Additionally added the round trip testing of the examples JSON-LD processing + schema validation

Closes #88

How was the issue fixed?

Briefly state why the change was necessary.

More context

List other areas that have changed but are not necessarily linked to the main feature. This could be naming changes,
bugs that were encountered and were fixed inline, etc.

@wolf4ood wolf4ood force-pushed the feat/88_usage_of_type_and_id branch 2 times, most recently from d2bff7f to 36d0a1e Compare December 6, 2024 10:27
@wolf4ood wolf4ood force-pushed the feat/88_usage_of_type_and_id branch from 36d0a1e to d5a15dc Compare December 6, 2024 10:46
@wolf4ood wolf4ood marked this pull request as ready for review December 6, 2024 11:54
@arnoweiss
Copy link
Contributor

arnoweiss commented Dec 6, 2024

This is great. One question: Are we actively deciding to keep the option between @type and type in the schemas? I'd feel more comfortable if we add "type": "@type" to the dcp context and remove @type from the schemas. That may however break non-json-ld-aware applications.

@wolf4ood
Copy link
Contributor Author

wolf4ood commented Dec 6, 2024

We already to the term redefinition for type to @type

https://github.com/eclipse-dataspace-dcp/decentralized-claims-protocol/blob/main/artifacts/src/main/resources/context/dcp.jsonld#L7-L9

It's fine for me to remove the @type from the JSON schema as long we clearly state that we do not support it.

Which app would break with type?

@jimmarino
Copy link
Contributor

@wolf4ood is there anything additional you need to do before this is merged?

@arnoweiss
Copy link
Contributor

I'll add another issue for removing the @type option from the schemas.

@jimmarino jimmarino merged commit 02f6f3f into eclipse-dataspace-dcp:main Dec 6, 2024
3 checks passed
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.

Usage of type and id
3 participants