-
-
Notifications
You must be signed in to change notification settings - Fork 244
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
Improve status updating mechanism #1210
Conversation
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.
From a code-perspective it LGTM 🚀 👍
I added some questions and comments here and there, and some notes to myself so that I don't forget to test certain things.
Speaking of which: Let me test those things and then I'll approve the PR, alright?
MastodonSDK/Sources/MastodonCore/FetchedResultsController/StatusDataController.swift
Show resolved
Hide resolved
MastodonSDK/Sources/MastodonCore/FetchedResultsController/StatusDataController.swift
Show resolved
Hide resolved
MastodonSDK/Sources/MastodonCore/FetchedResultsController/StatusDataController.swift
Show resolved
Hide resolved
MastodonSDK/Sources/MastodonCore/DataController/FeedDataController.swift
Outdated
Show resolved
Hide resolved
Mastodon/Scene/Thread/MastodonStatusThreadViewModel+State.swift
Outdated
Show resolved
Hide resolved
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.
Looks good great to me! 🚀
The last findings seem to work now, thank you very much for your work @kimar 🥳 💯
Rationale
The Status Refactoring PR removed the previous paradigm of how changes to the status data model are propagated throughout the app. This unfortunately also introduced bugs. This PR is making the status updates more transparent and fixes these bugs.