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

Removed warnings found in Swift 4.1 #34

Merged
merged 4 commits into from Apr 12, 2018
Merged

Removed warnings found in Swift 4.1 #34

merged 4 commits into from Apr 12, 2018

Conversation

KyeMaloy97
Copy link
Contributor

There are a low level API changes in Swift 4.1 which were giving warnings in other projects that depended on this repo, such as Kitura Websocket.

This PR just adds some conditional checks to use the relevant API depending on the Swift version.

Ran the test suite locally and built locally too on Swift 3.1.1, 4.0.3 and 4.1. I have added Travis builds for these Swift versions too.

@KyeMaloy97 KyeMaloy97 requested a review from billabt April 11, 2018 16:15
@codecov-io
Copy link

codecov-io commented Apr 11, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@7e24e6b). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #34   +/-   ##
=========================================
  Coverage          ?   76.72%           
=========================================
  Files             ?       10           
  Lines             ?      954           
  Branches          ?        0           
=========================================
  Hits              ?      732           
  Misses            ?      222           
  Partials          ?        0
Flag Coverage Δ
#Cryptor 76.72% <100%> (?)
Impacted Files Coverage Δ
Sources/Cryptor/StreamCryptor.swift 81% <100%> (ø)
Sources/Cryptor/HMAC.swift 70.86% <100%> (ø)
Sources/Cryptor/Digest.swift 93.18% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e24e6b...bb5fa2b. Read the comment docs.

@ianpartridge
Copy link
Contributor

You checked in the Xcode project.... Please remove it and add Cryptor.xcodeproj to .gitignore.

self.context.deallocate()
#else
self.context.deallocate(capacity: 1)
self.haveContext = false
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this line should be inside the #if block?

Copy link
Contributor

@ianpartridge ianpartridge left a comment

Choose a reason for hiding this comment

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

Hang on, I'm an idiot. I didn't realise this project has an Xcode project checked in already :(

In that case, I don't think we should touch it in this PR. @KyeMaloy97 can you remove the change to .gitignore and Cryptor.xcodeproj.

Just change .swift-version, Sources and .travis.yml in this PR.

Sorry.

@billabt
Copy link
Collaborator

billabt commented Apr 12, 2018

This PR looks fine to me. Will merge it. An' yes, it has an Xcode project. This is because there are variations on the build for different platforms that have been requested by users.

Copy link
Collaborator

@billabt billabt left a comment

Choose a reason for hiding this comment

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

This looks fine as is with the Xcode project.

@billabt billabt merged commit 7907ee3 into master Apr 12, 2018
@ianpartridge ianpartridge deleted the issue.4.1Updates branch April 12, 2018 12:59
billabt added a commit that referenced this pull request Apr 12, 2018
This reverts commit 7907ee3, reversing
changes made to 7e24e6b.
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

4 participants