Skip to content

add local contexts package#23

Merged
qqmyers merged 2 commits intogdcc:mainfrom
transfluxus:main
Jul 23, 2024
Merged

add local contexts package#23
qqmyers merged 2 commits intogdcc:mainfrom
transfluxus:main

Conversation

@transfluxus
Copy link
Copy Markdown
Collaborator

As discussed with @qqmyers this pull request contains all files for local contexts cvoc

Copy link
Copy Markdown
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

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

Cool! I made a few comments, none are critical, so unless you want to make changes for any of them, I'll go ahead and merge and we can deal with future changes (from whoever) as separate PRs. Just let me know when it's OK to merge.

Re: packages - I like the idea of keeping things together, but the scripts can nominally be used on different fields/several fields so it's not clear that packages is a good general way to organize. In any case, we don't have anything better for now, so, again, I can merge and think about this later.

Comment thread packages/local_contexts/lc-cvoc-conf.json Outdated
Comment thread packages/local_contexts/local_contexts.js Outdated
Comment thread packages/local_contexts/local_contexts.js Outdated
Comment thread packages/local_contexts/local_contexts.js Outdated
- reduced delay to fetch from LC API to 500ms
- cleaned solution to catch lc frame being added multiple times
- small formatting things
@transfluxus
Copy link
Copy Markdown
Collaborator Author

Re: packages - I like the idea of keeping things together, but the scripts can nominally be used on different fields/several fields so it's not clear that packages is a good general way to organize. In any case, we don't have anything better for now, so, again, I can merge and think about this later.

Ok, on our side this will be earliest be added in September. Anyway, if later, once there is a final location, I just need to know where it sits, so they can update their lc-cvoc-conf.json.

@qqmyers
Copy link
Copy Markdown
Member

qqmyers commented Jul 23, 2024

Re: packages - I like the idea of keeping things together, but the scripts can nominally be used on different fields/several fields so it's not clear that packages is a good general way to organize. In any case, we don't have anything better for now, so, again, I can merge and think about this later.

Ok, on our side this will be earliest be added in September. Anyway, if later, once there is a final location, I just need to know where it sits, so they can update their lc-cvoc-conf.json.

I'll try to remember to ping you in any PR for that. FWIW - in #22 updates to the README, I recommend that people deploy the scripts locally for production (perhaps creating a fork would work too). We do serve the dataverse-previewers from github.io, but trying to assure we never make changes that can affect someone's production instance is a pain and we now have several versions served in parallel since we don't know who may still be using the older ones.

@transfluxus
Copy link
Copy Markdown
Collaborator Author

Now I see again, why I had "allow-free-text": true, before. If it is set to false, I am not able to set that value, via the dv API

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