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

Build against Swift 5 #116

Merged
merged 2 commits into from
Dec 16, 2018
Merged

Build against Swift 5 #116

merged 2 commits into from
Dec 16, 2018

Conversation

djones6
Copy link
Contributor

@djones6 djones6 commented Dec 10, 2018

Adds testing with a Swift 5 development snapshot. The snapshot is defined in Travis as SWIFT_DEVELOPMENT_SNAPSHOT consistently across the IBM-Swift repos, to enable us to automate updating the version.

@djones6 djones6 requested review from ianpartridge and pushkarnk and removed request for ianpartridge December 10, 2018 16:32
@djones6
Copy link
Contributor Author

djones6 commented Dec 12, 2018

@pushkarnk could you review (particularly the change for ClientRequest) as I believe this alters its behaviour (its previous behaviour may have been wrong, but I suspect we don't have a test for this)

FYI the compilation warning that Swift 5 produces is this:

/Kitura-NIO/Sources/KituraNet/ClientRequest.swift:478:25: warning: string interpolation produces a debug description for a function value; did you mean to make this explicit?
        return "Basic \(data.base64EncodedString)"
                        ^~~~~~~~~~~~~~~~~~~~~~~~
/home/djones6/swift5/PRs/Kitura-NIO/Sources/KituraNet/ClientRequest.swift:478:30: note: use 'String(describing:)' to silence this warning
        return "Basic \(data.base64EncodedString)"
                        ~~~~~^~~~~~~~~~~~~~~~~~~
                        String(describing:      )

@pushkarnk
Copy link
Contributor

Right @djones6 .. that's a very awkward miss. There was no test and there were no compilation warnings prior to Swift 5. Thanks!

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.

2 participants