Skip to content

Feature/new fluree vocabulary #27

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

Merged
merged 9 commits into from
Oct 24, 2023
Merged

Conversation

dpetran
Copy link
Contributor

@dpetran dpetran commented Sep 11, 2023

This adds support for a new fluree vocabulary for https://github.com/fluree/core/issues/27

This builds off of the language tags branch as that has updated structure for #25.

@dpetran dpetran marked this pull request as draft September 11, 2023 20:50
It uses the "https://flur.ee" string as the key as that's the shortest possible one, but
we can change that in the future if we need to - the IRIs all expand to
"https://flur.ee/ns/<term>" regardless.
@dpetran dpetran force-pushed the feature/new-fluree-vocabulary branch from bef0bf6 to 806408c Compare September 14, 2023 18:34
@dpetran dpetran force-pushed the feature/new-fluree-vocabulary branch from 7a1bcd0 to 2be5f54 Compare October 6, 2023 17:27
@dpetran dpetran force-pushed the feature/new-fluree-vocabulary branch from 2198a13 to 2b4a4cf Compare October 24, 2023 01:08
@dpetran dpetran marked this pull request as ready for review October 24, 2023 13:05
@dpetran dpetran requested a review from a team October 24, 2023 13:05
Copy link
Contributor

@cap10morgan cap10morgan left a comment

Choose a reason for hiding this comment

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

Question about the meaning of a new docstring, but looks good o/w!

@@ -3,54 +3,76 @@
(:require ["jsonld" :as jldjs]
[fluree.json-ld.impl.external :as external]))

(defn pluggable-loader
"Takes a document-loader, which takes a url and options and returns a json string
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this missing a conclusion or a comma? In other words, does "and returns a json string..." apply to the document-loader (in which case this is missing a conclusion about what the fn itself returns / does) or to the fn itself (in which case this is missing a comma between "and options" and "and returns" (or you could wrap "which takes a url and options" in parens).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see how that's confusing! I tried rewriting it a few times and then decided to just document the argument explicitly, I think it's an improvement.

@dpetran dpetran merged commit c2bef37 into main Oct 24, 2023
@dpetran dpetran deleted the feature/new-fluree-vocabulary branch October 24, 2023 16:52
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