determined
determined copied to clipboard
determined-ai
chore: separate use ui store and dispatch
Description
these are intorduced as one more way to use a subset of our current store with the idea being that these would be shareable as opposed the Store as a whole being domain specific. without needing to update all the old references
Test Plan
Commentary (optional)
Checklist
- [ ] Changes have been manually QA'd
- [ ] User-facing API changes need the "User-facing API Change" label.
- [ ] Release notes should be added as a separate file under
docs/release-notes/. See Release Note for details. - [ ] Licenses should be included for new code which was copied and/or modified from any external code.
- [ ] If modifying
/webui/react/src/shared/verifymake -C webui/react test-sharedpasses.
Deploy Preview for storybook-det ready!
| Name | Link |
|---|---|
| Latest commit | 55f54639bdb2b3e27f6d0f954ac960577960e765 |
| Latest deploy log | https://app.netlify.com/sites/storybook-det/deploys/635872c454e9aa000804545b |
| Deploy Preview | https://deploy-preview-5032--storybook-det.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Deploy Preview for determined-ui ready!
| Name | Link |
|---|---|
| Latest commit | 55f54639bdb2b3e27f6d0f954ac960577960e765 |
| Latest deploy log | https://app.netlify.com/sites/determined-ui/deploys/635872c40109f10008d5e0ea |
| Deploy Preview | https://deploy-preview-5032--determined-ui.netlify.app/ |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
I need to move this pr to an upstream branch https://circleci.com/gh/determined-ai/determined/963572
thanks for the review.
This is looking good. Thoughts on using a separate context provider for the store information?
what do you mean by the store information? as in separating the provider for ui vs the rest?
If so that sounds good but if I'm understanding it correctly it wouldn't lend itself nicely to a transition period where not all modules that are using ui state are using useUI and we'd have to update all the references before introducing useUI
I marked the direct usage of StoreActionUI as deprecated to be updated separately
Yes, I meant a separate provider for ui. Yes, you would need to update the references. Is your plan here to update the references and then switch to another context provider in later PRs? If so that sounds fine to me.
@trentwatt and I were doing some work on splitting out agents into its own context (and adding proper loading states).
my plan was to not update the references here so I can land this sooner so we can use it sooner. but there aren't that many references and it'll let me remove a dependency on Store that was added previously unintentionally in the shared code so I think I should do it here
@hamidzr Okay. I'm amenable to either approach. If you update it in this PR can you ping me to re-review?
moving this to an upstream branch so the tests can run https://github.com/determined-ai/determined/pull/5338