-
Notifications
You must be signed in to change notification settings - Fork 35
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
Tweaks to DB API, improved abstractions for better testability. #913
Conversation
insertRootFolder(uuid: legacyFavoritesFolderID, into: context) | ||
} | ||
try prepareRootFolder(uuid: BookmarkEntity.Constants.rootFolderID) | ||
try prepareRootFolder(uuid: legacyFavoritesFolderID) | ||
} |
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.
This is now made to throw in case query fails.
dbFileURL: URL, | ||
errorEvents: EventMapping<MigrationErrors>? = nil) -> [String]? { | ||
public func getFavoritesOrderFromPreV4Model(dbContainerLocation: URL, | ||
dbFileURL: URL) throws -> [String]? { |
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.
Instead of using event mapper to handle errors, just throw it from here and handle in client.
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.
LGTM, bashed on this a fair bit over the week and had no issues with the client testing steps. Thanks!
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/856498667320406/1207875839908777/f
iOS PR: duckduckgo/iOS#3143
macOS PR: duckduckgo/macos-browser#3033
What kind of version bump will this require?: Major
Description:
Tweak code to improve error handling and events propagation in Clients. Add protocols for improved testability.
Steps to test this PR:
See platform PRs.
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template