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

Remove internal _isDatabaseObserverItemReusingEnabled check #718

Merged
merged 3 commits into from
Jan 17, 2025

Conversation

laevandus
Copy link
Contributor

@laevandus laevandus commented Jan 16, 2025

🎯 Goal

Remove _isDatabaseObserverItemReusingEnabled check which has been enabled for 6 months without any side-effects

🛠 Implementation

  • Remove the check and delete any fallback code
  • Remove message caching because it was turned off with that check before
  • Deprecate public userDisplayInfo(from id: String) -> UserDisplayInfo? method which is not used and only accessed cached data

🧪 Testing

N/A

☑️ Checklist

  • I have signed the Stream CLA (required)
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Affected documentation updated (docusaurus, tutorial, CMS (task created)

@laevandus laevandus requested a review from a team as a code owner January 16, 2025 08:28
…reat it as true which allows removing most of the message caching
@laevandus laevandus force-pushed the cleanup/model-reuse-runtime-check branch from 122b0dd to f7040b4 Compare January 16, 2025 08:32
Copy link

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Jan 16, 2025

SDK Size

title develop branch diff status
StreamChatSwiftUI 7.54 MB 7.55 MB +15 KB 🟢

Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

Would be great to do some performance checks before merging this.

@laevandus
Copy link
Contributor Author

Would be great to do some performance checks before merging this.

Ran it with time profiler as well. Nothing shows up related to these changes. That said, I saw some DataStore accesses on the main thread, but this is a separate story.

@laevandus laevandus enabled auto-merge (squash) January 17, 2025 09:54
@laevandus laevandus merged commit 1effcd5 into develop Jan 17, 2025
12 checks passed
@laevandus laevandus deleted the cleanup/model-reuse-runtime-check branch January 17, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants