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 `substring` API warnings for Swift 3.2 and up #2240

Merged
merged 2 commits into from Aug 22, 2017

Conversation

Projects
None yet
4 participants
@htinlinn
Contributor

htinlinn commented Aug 13, 2017

As of the latest version of Xcode 9 beta, substring functions from String have been deprecated and show up as warnings when compiling using Swift 4 compiler.

This pull request fixes those warnings by replacing calls to those functions with the new subscript slicing syntax.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost commented Aug 15, 2017

thanks

@jshier

One logic change and some whitespace changes, otherwise this looks good. Thanks!

Show outdated Hide outdated Source/Request.swift
Show outdated Hide outdated Source/Request.swift
Show outdated Hide outdated Source/Request.swift
@cnoon

This comment has been minimized.

Show comment
Hide comment
@cnoon

cnoon Aug 22, 2017

Member

Hi @htinlinn,

The compiler warnings you mention here do not appear for me in Xcode 9 beta 5. I'm currently installing beta 6 and will give it a shot there, but I don't anticipate we'll see the warnings there either. Were you seeing these issues in beta 4?

I'm going to close issue out for now since I'm not seeing the warnings. We'll certainly take a pass at updating all the APIs and implementations to the latest Swift 4 syntax in the AF 5 branch.

If I'm completely mistaken here and the warnings are still present in beta 5 and for some reason I'm not seeing them, then please comment back on this ticket and we'll happily re-open.

Thanks anyways! 🍻

Member

cnoon commented Aug 22, 2017

Hi @htinlinn,

The compiler warnings you mention here do not appear for me in Xcode 9 beta 5. I'm currently installing beta 6 and will give it a shot there, but I don't anticipate we'll see the warnings there either. Were you seeing these issues in beta 4?

I'm going to close issue out for now since I'm not seeing the warnings. We'll certainly take a pass at updating all the APIs and implementations to the latest Swift 4 syntax in the AF 5 branch.

If I'm completely mistaken here and the warnings are still present in beta 5 and for some reason I'm not seeing them, then please comment back on this ticket and we'll happily re-open.

Thanks anyways! 🍻

@cnoon cnoon closed this Aug 22, 2017

@cnoon cnoon added the swift label Aug 22, 2017

@jshier

This comment has been minimized.

Show comment
Hide comment
@jshier

jshier Aug 22, 2017

Contributor

@cnoon They only appear in Swift 4 mode.

@htinlinn One last change and this should be good to go.

Contributor

jshier commented Aug 22, 2017

@cnoon They only appear in Swift 4 mode.

@htinlinn One last change and this should be good to go.

@jshier jshier reopened this Aug 22, 2017

@htinlinn

This comment has been minimized.

Show comment
Hide comment
@htinlinn

htinlinn Aug 22, 2017

Contributor

@jshier Done. Sorry about all the back and forth with this. I wasn't thinking about newlines when you said whitespace.

Contributor

htinlinn commented Aug 22, 2017

@jshier Done. Sorry about all the back and forth with this. I wasn't thinking about newlines when you said whitespace.

@cnoon

This comment has been minimized.

Show comment
Hide comment
@cnoon

cnoon Aug 22, 2017

Member

Ah thanks @jshier...late night. I figured I was missing something. 😕

Member

cnoon commented Aug 22, 2017

Ah thanks @jshier...late night. I figured I was missing something. 😕

@jshier

jshier approved these changes Aug 22, 2017

@jshier

This comment has been minimized.

Show comment
Hide comment
@jshier

jshier Aug 22, 2017

Contributor

Merging without full Travis, because it would likely be hours before Travis got to us. All tests pass locally for me.

Contributor

jshier commented Aug 22, 2017

Merging without full Travis, because it would likely be hours before Travis got to us. All tests pass locally for me.

@jshier jshier merged commit e396a0e into Alamofire:master Aug 22, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

@cnoon cnoon added this to the 4.5.1 milestone Sep 6, 2017

@PGRBryant

This comment has been minimized.

Show comment
Hide comment
@PGRBryant

PGRBryant Mar 9, 2018

Looks like we've still got three substring deprecated and one 'characters' deprecated warnings hanging around. Unless the version I'm running from cocoapads isn't up to date?

PGRBryant commented Mar 9, 2018

Looks like we've still got three substring deprecated and one 'characters' deprecated warnings hanging around. Unless the version I'm running from cocoapads isn't up to date?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment