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

Modularize schema #24

Merged
merged 2 commits into from
Oct 10, 2020
Merged

Modularize schema #24

merged 2 commits into from
Oct 10, 2020

Conversation

acka47
Copy link
Member

@acka47 acka47 commented Oct 9, 2020

I did a first quick run at the modularization and made some dirty changes to the test script so that it will run. @sroertgen please take a look at it whether it goes in the right direction.

acka47 added 2 commits October 9, 2020 15:06
...use links to raw files in branch as reference for testing.
(somebody will have to fix it properly later)
@acka47 acka47 requested a review from sroertgen October 9, 2020 13:32
@acka47
Copy link
Member Author

acka47 commented Oct 9, 2020

I only took a quick look at the editor but from what I saw it seems to be running as expected: https://skohub.io/editor/?schema=https://raw.githubusercontent.com/dini-ag-kim/lrmi-profile/23-modularize/draft/schemas/schema.json

Copy link
Contributor

@sroertgen sroertgen left a comment

Choose a reason for hiding this comment

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

I suggest few changes in context-schema.json, the language part does not work as expected (master has this problem as well I noticed).

I tried with the suggested changes in my fork and then publishing theoretically works, but I get an error from skohub saying it can't find the respective inbox. Which is strange, because the items shows up in the inbox. But this seems to be another topic, have to open an issue for this.

"type": "object",
"properties": {
"@language": {
"$ref": "https://raw.githubusercontent.com/dini-ag-kim/lrmi-profile/23-modularize/draft/schemas/Language.json"
Copy link
Contributor

@sroertgen sroertgen Oct 10, 2020

Choose a reason for hiding this comment

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

This does not seem to work as expected. There is no field added to select the language for @context.

When trying to publish something, it throws an error because we have "minItems": 2, but the field for language selection for "@context" does not show up in the editor.
I recommend setting "minItems": 1 and required to an empty array for now till this is fixed

},
"additionalProperties": false,
"required": [
"@language"
Copy link
Contributor

Choose a reason for hiding this comment

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

as mentioned above I suggest setting this to []

"title": "JSON-LD Context",
"description": "The JSON-LD context for the structured resource descriptions",
"type": "array",
"minItems": 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

as mentioned below I suggest setting this to 1

@acka47
Copy link
Member Author

acka47 commented Oct 10, 2020

There is no field added to select the language for @context.

See skohub-io/skohub-editor#32 for this.

When trying to publish something, it throws an error because we have "minItems": 2, but the field for language selection for "@context" does not show up in the editor.

I assume you are talking about publishing a description to a skos:Concept. Currently, the payload is not validated against any schema, so you can basically publish whatever you want to an inbox, it just has to be JSON-LD. So there must be another problem.

I suggest to discuss SKoHub Pubsub related issues at https://github.com/hbz/skohub-pubsub/issues. Please open an issue over there including a link to an inbox you have set up.

@sroertgen
Copy link
Contributor

with #25 this will work.

@sroertgen sroertgen merged commit 4abeb03 into master Oct 10, 2020
@acka47 acka47 deleted the 23-modularize branch January 8, 2021 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants