-
Notifications
You must be signed in to change notification settings - Fork 208
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
Rework Workspace and Settings apis #6646
base: master
Are you sure you want to change the base?
Conversation
full-stack-tests/backend/src/integration/CloudWorkspace.test.ts
Outdated
Show resolved
Hide resolved
…twinjs-core into load-workspace-resources
This reverts commit d0de539.
*/ | ||
getObject<T extends object>(settingName: SettingName, defaultValue: T): T; | ||
getObject<T extends object>(settingName: SettingName): T | undefined; | ||
|
||
/** Get an array setting by SettingName. | ||
* @param settingName The name of the setting | ||
* @param defaultValue value returned if settingName is not present in any SettingDictionary, or if the highest priority setting is not an array. | ||
* @param defaultValue value returned if settingName is not present in any Settings.Dictionary, or if the highest priority setting is not an array. |
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.
I just noticed that these comments are now wrong. It throws if the value doesn't match.
I meant to reply to this comment:
VS Code does that for objects, but I strongly suggest we don't allow it. It creates a big complicated, ambiguous mess for queries. I don't see it being worthwhile or necessary. |
Fine by me. I will rename |
Some questions from @Josh-Schifter below. I am starting on rewriting Workspace.md now, with an eye toward making sure it provides answers to all of these (among others).
|
This pull request is now in conflicts. Could you fix it @kabentley? 🙏 |
Make the apis for loading Settings and WorkspaceResources easier to use. It makes it possible to store SettingsDictionaries in cloud workspaces and load them automatically when you open an iModel.
This includes breaking changes to the (beta) apis for:
It also introduces the Workspace.Editor namespace that can be used to modify Workspaces.
@johnnyd710 this will require work for the Settings Editor. It enforces that all Settings defined by a SettingSchema must all have the same prefix. That will undoubtedly break existing code in iTS, since settings will have a different name. You also need to add the concept of storing SettingsDictionaries in cloud containers, as we discussed.
Also, @johnnyd710, @wgoehrig, @MichaelSwigerAtBentley we should talk about how the settings editor will allow picking cloud-based WorkspaceDbs. We will probably need some kind of queries from the BlobContainer service.