Skip to content
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

Fixed Xcode 10.2 warnings #136

Merged
merged 1 commit into from
May 21, 2019
Merged

Fixed Xcode 10.2 warnings #136

merged 1 commit into from
May 21, 2019

Conversation

RomanPodymov
Copy link
Contributor

Hello.
In this pull request I added @unknown default: to the implementation of DispatchQoS.QoSClass.description. It works fine with all supported Swift version in Xcode 10.2 (see this link for more details about @unknown).
I added QoSClassDescription to unify the string constants using in DispatchQoS.QoSClass.description and qos_class_t.description.
I also removed get in some computed properties.

@duemunk
Copy link
Owner

duemunk commented May 20, 2019

Thanks for creating this!

I see no reason to unify with a mirror enum for DispatchQoS.QoSClass. Could you remove that and just keep the @unkown default?

@RomanPodymov
Copy link
Contributor Author

RomanPodymov commented May 20, 2019

Hello @duemunk
Currently AsyncSwift.swift is full of duplicate strings. I tried to solve this problem with an enum with associated values. DispatchQoS.QoSClass and qos_class_t are related to each other and QoSClassDescription demonstrates it. And when a new case will be added to DispatchQoS.QoSClass you won't add a new description twice. Please rename QoSClassDescription and propose a better description for it if you want.

@RomanPodymov
Copy link
Contributor Author

Hello @duemunk
I reverted my changes to f6be54a where I fixed Xcode 10.2 warnings. I put QoSClassDescription to another branch, so we can discuss this feature later.

@duemunk duemunk merged commit 669c88a into duemunk:master May 21, 2019
@duemunk
Copy link
Owner

duemunk commented May 21, 2019

Perfekt @RomanPodymov!

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.

None yet

2 participants