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

Renamed indexes(of:) to indices(of:) in ArrayExtension #355

Merged
merged 3 commits into from
Jan 4, 2018

Conversation

Najdan
Copy link
Member

@Najdan Najdan commented Jan 4, 2018

Summary

Renamed method of ArrayExtensions indexes(of:) to indices(of:) since Apple's API is using that naming. There are few other usages in SwifterSwift project where "indices" is used and this was only place where "indexes" was used.

Checklist

  • I checked the Contributing Guidelines before creating this request.
  • New extensions are written in Swift 4.
  • New extensions support iOS 8.0+ / tvOS 9.0+ / macOS 10.10+ / watchOS 2.0+.
  • I have added tests for new extensions, and they passed.
  • All extensions have a clear comments explaining their functionality, all parameters and return type in English.
  • All extensions are declared as public.

@SwifterSwiftBot
Copy link

SwifterSwiftBot commented Jan 4, 2018

1 Warning
⚠️ Consider adding tests for new extensions or updating existing tests for a modified SwifterSwift extension
1 Message
📖 Thank you for submitting a pull request to SwifterSwift. The team will review your submission as soon as possible.

Generated by 🚫 Danger

@codecov
Copy link

codecov bot commented Jan 4, 2018

Codecov Report

Merging #355 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #355   +/-   ##
=======================================
  Coverage   90.62%   90.62%           
=======================================
  Files          53       53           
  Lines        2508     2508           
=======================================
  Hits         2273     2273           
  Misses        235      235
Flag Coverage Δ
#ios 90.62% <100%> (ø) ⬆️
#osx 86.92% <100%> (ø) ⬆️
#tvos 86.92% <100%> (ø) ⬆️
Impacted Files Coverage Δ
...urces/Extensions/SwiftStdlib/ArrayExtensions.swift 100% <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 215516a...38586f6. Read the comment docs.

@omaralbeik omaralbeik self-requested a review January 4, 2018 06:37
Copy link
Member

@omaralbeik omaralbeik left a comment

Choose a reason for hiding this comment

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

Thank you @Najdan, I agree, it makes more sense now 💯

Could you move the deprecated function into SwiftStdlib/Deprecated/SwiftStdlibDeprecated.swift, and I'll make sure to get this merged in as soon as I can.

@Najdan
Copy link
Member Author

Najdan commented Jan 4, 2018

Sure. I pushed new commit with requested changes.

@Najdan
Copy link
Member Author

Najdan commented Jan 4, 2018

@omaralbeik wait before you close this pull request. I've noticed that I forgot to update Examples.md. I'll check if it's used somewhere else too.

Examples.md contained indexes(of:) which is renamed to indices(of:)
@Najdan
Copy link
Member Author

Najdan commented Jan 4, 2018

@omaralbeik Sorry about this late additional commit. I didn't noticed on time because this file is not included in Xcode project. I searched all files in project to see if anywhere else "indexes" is used and it looks like only deprecations are using "indexes" (test is still available for old indexes(of:) method but I think it should stay for now).

@omaralbeik omaralbeik merged commit 65f87a7 into SwifterSwift:master Jan 4, 2018
@omaralbeik
Copy link
Member

Thank you @Najdan 👍

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.

3 participants