Reduce need for mutable App access, towards decoupling with the ViewerContext#2333
Closed
Reduce need for mutable App access, towards decoupling with the ViewerContext#2333
Conversation
…ted recording & blueprint
emilk
reviewed
Jun 8, 2023
| strum = { version = "0.24", features = ["derive"] } | ||
| strum_macros = "0.24" | ||
| sublime_fuzzy = "0.7" | ||
| crossbeam.workspace = true |
| selected_rec_id: RwLock<Option<StoreId>>, | ||
| #[serde(skip)] | ||
| pub(crate) selected_blueprint_by_app: HashMap<ApplicationId, StoreId>, | ||
| selected_blueprint_by_app: RwLock<HashMap<ApplicationId, StoreId>>, |
Member
There was a problem hiding this comment.
I'm not sure this is a good solution. It's nice that &AppState means read-only, and adding interior mutability to everything is quite unidiomatic, and an ugly way to circumvent the borrowchecker when we should not need to circumvent it at all
Merged
1 task
Wumpf
added a commit
that referenced
this pull request
Jun 8, 2023
…ithout mutable access (#2339) <!-- Open the PR up as a draft until you feel it is ready for a proper review. Do not make PR:s from your own `main` branch, as that makes it difficult for reviewers to add their own fixes. Add any improvements to the branch as new commits to make it easier for reviewers to follow the progress. All commits will be squashed to a single commit once the PR is merged into `main`. Make sure you mention any issues that this PR closes in the description, as well as any other related issues. To get an auto-generated PR description you can put "copilot:summary" or "copilot:walkthrough" anywhere. --> ### What Allows for decoupling a bunch of things! The good parts from #2333 ;) ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) <!-- This line will get updated when the PR build summary job finishes. --> PR Build Summary: https://build.rerun.io/pr/2339 <!-- pr-link-docs:start --> Docs preview: https://rerun.io/preview/c58f5af/docs Examples preview: https://rerun.io/preview/c58f5af/examples <!-- pr-link-docs:end --> --------- Co-authored-by: Jeremy Leibs <jeremy@rerun.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
In hope to help out #2330, but doesn't nearly go far enough yet.
Interior mutability for App's pending commands, app options and selected recording & blueprint
Checklist
PR Build Summary: https://build.rerun.io/pr/2333
Docs preview: https://rerun.io/preview/e95997a/docs
Examples preview: https://rerun.io/preview/e95997a/examples