Skip to content

Strframework 1674#101

Merged
Tomusm merged 9 commits intomasterfrom
STRFRAMEWORK-1674
Jan 31, 2020
Merged

Strframework 1674#101
Tomusm merged 9 commits intomasterfrom
STRFRAMEWORK-1674

Conversation

@sheenvempeny
Copy link
Copy Markdown

@sheenvempeny sheenvempeny commented Jan 30, 2020

https://smartmobilefactory.atlassian.net/browse/STRFRAMEWORK-1674

Plist2Swift was not handling the optional variable (keys are not present in all the plists) inside a dictionary. This PR will fix that issue.

WAIT BEFORE MERGING

@sheenvempeny sheenvempeny self-assigned this Jan 30, 2020
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
@proth proth requested a review from kmeinh January 30, 2020 10:43
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift
@Tomusm Tomusm removed the request for review from rbugajewski January 30, 2020 17:34
Copy link
Copy Markdown
Contributor

@rbugajewski rbugajewski left a comment

Choose a reason for hiding this comment

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

Just some small typos, otherwise lgtm 👍🏻

Comment thread Plist2swift/Plist2swift.swift
Comment thread Plist2swift/Plist2swift.swift
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
Comment thread Plist2swift/Plist2swift.swift Outdated
@Tomusm
Copy link
Copy Markdown
Contributor

Tomusm commented Jan 30, 2020

@sheenvempeny I tested plist2Swift on the 3.0.1/master of the mac app.
It technically works, but It makes too much optionals.

For example here:

	internal struct EndPointAuthorize: EndPointAuthorizeProtocol {
		internal var absoluteURL: String? = "https://www.hidrive.strato.com/oauth2/authorize"
		internal var uRLFragment: String? = "login"
		internal var uRLParamResponseType: String? = "code"
		internal var uRLParamScope: String? = "admin,rw"
	}

	var endPointAuthorize: EndPointAuthorizeProtocol? {
		return EndPointAuthorize()
	}

Technically here, only var endPointAuthorize: EndPointAuthorizeProtocol? should be optional. The content of the struct does not have to in this case.
If we would add the "redirectURL" parameter from the staging, then only this redirectURL parameter would need to be optional since it"s not present on the live environment.
You can test plist2swift on the mac app master. It would be good if the update do not prevent the project to build.

Comment thread Plist2swift/Plist2swift.swift Outdated
@Tomusm Tomusm removed the request for review from rbugajewski January 31, 2020 12:11
@Tomusm Tomusm merged commit 7d6e6e3 into master Jan 31, 2020
@Tomusm Tomusm deleted the STRFRAMEWORK-1674 branch January 31, 2020 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants