Feature/translation api by name #9
Merged
Conversation
This file contains 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
added 9 commits
November 27, 2017 16:05
…/MikePT28/halo-ios into feature/translation-api-by-name * 'feature/enable_change_credential' of https://github.com/MikePT28/halo-ios: [CoreManager]: update how to setup app credentials
…/MikePT28/halo-ios into feature/translation-api-by-name * 'feature/enable_change_credential' of https://github.com/MikePT28/halo-ios: [Coremanager]: delete unused private functions
…/MikePT28/halo-ios into feature/translation-api-by-name * 'feature/enable_change_credential' of https://github.com/MikePT28/halo-ios: [Core]: update core to allow editing environment
MikePT28
requested changes
Nov 30, 2017
| @@ -18,7 +18,7 @@ extension ContentManager { | |||
|
|
|||
| public func sync(query: SyncQuery, completionHandler handler: @escaping (String, HaloError?) -> Void) -> Void { | |||
|
|
|||
| let path = getPath(file: "synctimestamp-\(query.moduleId)") | |||
| let path = getPath(file: "synctimestamp-\(query.moduleId!)") | |||
There was a problem hiding this comment.
If there's no moduleId the string would read: "synctimestamp-nil" or the worst case is a thread access error.
Please wrap it into a guard so that if there's no moduleId return a HaloError to the user
| // Copyright © 2017 MOBGEN Technology. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation |
There was a problem hiding this comment.
Move as a second extension on the ContentManager+Sync.swift file
| @@ -21,14 +21,16 @@ open class SyncQuery: NSObject { | |||
|
|
|||
| open fileprivate(set) var locale: Locale = Manager.content.defaultLocale | |||
| open fileprivate(set) var moduleName: String? | |||
| open fileprivate(set) var moduleId: String = "" | |||
| open fileprivate(set) var moduleId: String? | |||
There was a problem hiding this comment.
Change ? to ! and make sure it is never nil when used
| if let id = moduleId { | ||
| Manager.content.sync(query: syncQuery) { moduleId, error in | ||
| self.processSyncResult(moduleId: id, error: error) | ||
| self.completionHandlers.forEach { $0(error) } |
There was a problem hiding this comment.
To avoid duplicating this line, make an inline closure/block/lambda and call it here and on the other place where used
| } | ||
|
|
||
| if let username = data[usernameKey] as? String, let password = data[passwordKey] as? String { | ||
| self.userCredentials = Credentials(username: username, password: password) | ||
| if(self.appCredentials != nil) { |
There was a problem hiding this comment.
Change to userCredentials
Sources/Managers/CoreManager.swift
Outdated
| } | ||
|
|
||
| if let tags = data[CoreConstants.enableSystemTags] as? Bool { | ||
| self.enableSystemTags = tags | ||
| } | ||
|
|
||
| if let env = data[environmentKey] as? String { | ||
| if(self.env != nil) { |
Sources/Managers/CoreManager.swift
Outdated
|
|
||
| - parameter env: The env to set | ||
| */ | ||
| public func setEndpoint(_ env: String) { |
There was a problem hiding this comment.
Remove this setter and change it's uses to direct setting of the var.
added 4 commits
November 30, 2017 16:18
…/MikePT28/halo-ios into feature/translation-api-by-name * 'feature/enable_change_credential' of https://github.com/MikePT28/halo-ios: Update due to PR review comments
MikePT28
approved these changes
Nov 30, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Get translations by module name