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

allow users to clear the default context for queries #422

Closed
wants to merge 1 commit into from

Conversation

dpetran
Copy link
Contributor

@dpetran dpetran commented Mar 22, 2023

This is a partial fix for #395, allowing users to supply an explicitly nil context to clear the default context.

This is just the bare minimum changes necessary to support the new behavior of the json-ld dependency.

@bplatz
Copy link
Contributor

bplatz commented Apr 5, 2023

With the approach taken with PR #437, now the default behavior is to always clear default context. I don't think this PR is needed as it stands, but perhaps parts of it should still be included.

@dpetran
Copy link
Contributor Author

dpetran commented Apr 6, 2023

We still need this for correct context handling (if someone passes us a bunch of contexts interspersed with nils, we'll need to handle that case) and for a fix to #395, I think. I'll try to see how to rebase on top of the existing context work.

@bplatz
Copy link
Contributor

bplatz commented Apr 6, 2023

We still need this for correct context handling (if someone passes us a bunch of contexts interspersed with nils, we'll need to handle that case) and for a fix to #395, I think. I'll try to see how to rebase on top of the existing context work.

Sure, just would say its not high priority if it take any amount of work. If someone doesn't want to include a context, they can simply not include it. Before with our default context people didn't have that option, so it was needed. But now they have that option so it isn't a significant issue short-term.

@dpetran
Copy link
Contributor Author

dpetran commented Apr 6, 2023

Well, I just tried using the json-ld from fluree/json-ld#14 directly on main and it didn't break any tests, so I don't think this PR is needed anymore, but I do think we should merge that other one. That would solve #395 at least.

@dpetran dpetran closed this Apr 6, 2023
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