Finished conversion to Swift 4 #57
Conversation
|
|
|
Great! I'll review and re-run Travis some time today. Travis build won't pass unless I do that. (We need to change this) |
Sources/SwiftSMTP/Sender.swift
Outdated
| @@ -149,7 +149,7 @@ extension String { | |||
| throw SMTPError(.createEmailRegexFailed) | |||
| } | |||
|
|
|||
| let range = NSRange(location: 0, length: self.characters.count) | |||
| let range = NSRange(location: 0, length: self.count) | |||
There was a problem hiding this comment.
You could remove self in self.count here if you wanted.
There was a problem hiding this comment.
Does IBM have any Swift style guidelines I could follow in the future? I understand that Swift typically advises against using "self", but didn't change it in this instance as it was used in "self.characters.count".
There was a problem hiding this comment.
In general we've followed https://github.com/raywenderlich/swift-style-guide although it's a bit out of date now. Removing self. in this case is a good idea :)
|
Looking into how to get the Travis build working again. |
|
Hmm, still failing. @quanvo87 will try another tactic... |
|
@ianpartridge @quanvo87 you could always just take my word that it works? Who needs thorough testing, right? |
|
That's half the fun :) |
|
I've implemented a new dev ops flow. It is an improvement over the old one, but we still cannot get around the fact that Travis repo env vars are not available to forks (for good reason). So for now, I've pulled this PR into a PR of my own, #58. Tests pass because my PR is not a fork and Travis can access the repo env vars. I'll merge and provide credit to @Davidde94 in the release notes. Future considerations for dev ops improvements could be:
|
|
Thanks for doing this @quanvo87 ! |
|
Thanks :) |
|
@quanvo87 re: others? Change unit tests to use a botnet spam relay, so no auth is required? |
Conversion to Swift 4.0.3 has been finalised, with all warnings removed.
Motivation and Context
The warnings were irritating me across several projects.
How Has This Been Tested?
Using the already-existing test suite
Checklist: