-
Notifications
You must be signed in to change notification settings - Fork 21
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 README.md text to help others host index meta-databases #18
Merged
Merged
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
62c3f7d
Update README.md about how to use repo to host your own index meta-da…
rartino 7328dde
Minor text fixes in README.md
rartino dbb28ea
Minor README.md text fixes
rartino e7c0280
Apply suggestions from code review
rartino 909254b
Merge master into update_readme
rartino 0c7c4a5
Adapt instructions to latest specification changes.
rartino 3737786
Merge remote-tracking branch 'origin/master' into update_readme
CasperWA File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is new!
I like it - but it defeats our idea of "supporting" only two levels (as we write in the spec here) - whatever support means in this context.
I thought the way to include the providers, was simply to include them with the type
provider
, again, as is indicated in the spec here.However! I am all for not replicating too much, i.e., having the same providers spread out over all the different implementations. And having the option to direct the client to a continuously updated list of providers seems to be a good one to me.
My only issue with this then, is actually how we should present this link to the list of providers. While this may be one way, it is a bit misinformative - since the list of providers is not a parent of the index meta-database, but rather it's on the same level (in some sense).
Another option would be to introduce a fourth type, like the plural of provider, i.e.,
"providers"
or a combined type of"provider_list"
.Another option would be to introduce a type that refers to implementation on the same "level", like
"sibling"
(to follow on with thechild
andparent
nomenclature), but that feels a bit mis-informative as well, I feel.So in the end, I am open to ideas, and it may even end up with what you have here. I would just like to raise the discussion to make sure it all makes sense with the rest of the spec (or adapt the spec accordingly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh... I had somehow gotten the idea that this was how it was supposed to work so that an unbroken tree was presented to the client, which could be traversed both up and down. But you are right that the present specification certainly do not mandate this setup. In particular, the fact that providers are linked out with
provider
-type objects instead ofchild
already breaks this symmetry.So, for now I should just remove this segment. But I like your version with a
provider_list
link.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, maybe my confusion ties into Materials-Consortia/OPTIMADE#217 a bit. Should these link objects really have these kinds of different types, or would it be better to change them to be
links
-type and put the relationship as an attribute field?