Skip to content
This repository has been archived by the owner. It is now read-only.

Migrating tabs and bookmarks from Ghostery #28

Merged
merged 1 commit into from May 9, 2018

Conversation

@mahmoud-adam85
Copy link
Contributor

@mahmoud-adam85 mahmoud-adam85 commented May 3, 2018

No description provided.

self.delegate = migrationDelegate

let defaults = UserDefaults.standard
guard defaults.object(forKey: GhosteryMigrationManager.migrationKey) == nil && self.ghosteryDB != nil else {

This comment has been minimized.

@naira-cliqz

naira-cliqz May 8, 2018
Contributor

Would be better to check for the migrationKey in init method, before opening the DB do not initialize the DB if it's already migrated. In startMigration it will start if the db is not nil.

DispatchQueue.global().async { [weak self] in
self?.migrateOpenTabs()

let migrateFolders = self?.migrateBookmarkFolders()

This comment has been minimized.

@naira-cliqz

naira-cliqz May 8, 2018
Contributor

Would be better to have another method for this chunk like migrateFoldersAndBookmarks and migrateBookmarkFolders this method can be just migrateFolders

import Deferred

public protocol GhosteryMigrationDelegate: class {
func migrationOpenURLInNewTab(_ url: URL)

This comment has been minimized.

@naira-cliqz

naira-cliqz May 8, 2018
Contributor

the method name could be more generic, like ghosteryTabMigrated

}
}

private func migrateBookmarkFolders() -> Deferred<Maybe<Cursor<BookmarkMirrorItem>>> {

This comment has been minimized.

@naira-cliqz

naira-cliqz May 8, 2018
Contributor

Actually this method doesn't migrate the data, it only fetches the data

}

private func migrateOpenTabs() {
let sql = "SELECT * from \(TableGhosteryTabs)"

This comment has been minimized.

@naira-cliqz

naira-cliqz May 8, 2018
Contributor

why query isn't ordered by instead of sorting afterwards?

@mahmoud-adam85 mahmoud-adam85 force-pushed the mahmoud-adam85:ghostery-migration branch 2 times, most recently from 980fc81 to 55eb65c May 8, 2018
@mahmoud-adam85 mahmoud-adam85 force-pushed the mahmoud-adam85:ghostery-migration branch from 55eb65c to 08c6290 May 9, 2018
@mahmoud-adam85 mahmoud-adam85 merged commit f0c3411 into ghostery:master May 9, 2018
1 check was pending
1 check was pending
@cliqz-oss-ci
continuous-integration/jenkins/pr-merge This commit is being built
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants