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

Minor crash resolved #473

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Minor crash resolved #473

wants to merge 1 commit into from

Conversation

ioskunal
Copy link

invalid check mistake resolved

Checklist

  • New Extension
  • New Test
  • Changed more than one extension, but all changes are related
  • Trivial change (doesn't require changelog)

invalid check mistake resolved
@Esqarrouth
Copy link
Owner

Esqarrouth commented Mar 22, 2018

Thanks for the PR. Is there an associated test with this? Did it run before? Does it run now?

@ioskunal
Copy link
Author

ioskunal commented Mar 22, 2018

Thanks for quick reply. I tested this manually. The loop has to be executed from 0 to n-1 where as it runs from 0 to n and then it crashes at n index. I had checked this and now it works fine.

@EZSwiftExtensionsBot
Copy link

1 Error
🚫 Making non-trivial change requires changelog entry! Please, set trivial change or add entry to changelog.
2 Messages
📖 Executed 203 tests, with 0 failures (0 unexpected) in 6.527 (6.789) seconds
📖 Executed 125 tests, with 0 failures (0 unexpected) in 4.777 (4.869) seconds

Generated by 🚫 Danger

@Esqarrouth
Copy link
Owner

Could you edit the test cases here so it gives an asset while running the code:

for i in 0...length {

But doesn't give an assert with:

for i in 0..<length {

https://github.com/goktugyil/EZSwiftExtensions/blob/master/EZSwiftExtensionsTests/StringTests.swift

@@ -438,7 +438,7 @@ extension String {

/// EZSE: Checks if String contains Emoji
public func includesEmoji() -> Bool {
for i in 0...length {
for i in 0..<length {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you write a unit test for this pls ?

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