Skip to content

Feature/new fluree vocabulary#27

Merged
dpetran merged 9 commits intomainfrom
feature/new-fluree-vocabulary
Oct 24, 2023
Merged

Feature/new fluree vocabulary#27
dpetran merged 9 commits intomainfrom
feature/new-fluree-vocabulary

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!

[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