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

Added examples/docs to split in str.rs #33893

Merged
merged 1 commit into from
May 30, 2016
Merged

Added examples/docs to split in str.rs #33893

merged 1 commit into from
May 30, 2016

Conversation

Ophirr33
Copy link
Contributor

@Ophirr33 Ophirr33 commented May 26, 2016

Added documentation clarifying the behavior of split when used with the empty string and contiguous separators. Addresses issue 33882. This is my first time contributing to rust, so forgive me if I'm skipping any of the contribution steps.
Fixes #33882

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gankro (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@steveklabnik
Copy link
Member

Looks great! If you say "Fixes #33882" in your commit message, it will close the issue when this gets merged. Would you mind amending the message to put this in?

@Ophirr33
Copy link
Contributor Author

Absolutely, just fixed that. Thanks for the heads up!

On Thu, May 26, 2016 at 6:57 PM, Steve Klabnik notifications@github.com
wrote:

Looks great! If you say "Fixes #33882
#33882" in your commit message,
it will close the issue when this gets merged. Would you mind amending the
message to put this in?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#33893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/APLtwS5PBtuecyjXSakLVT2dSlrvBuXRks5qFiVmgaJpZM4IoBuH
.

@steveklabnik
Copy link
Member

I am not seeing the fix, did you push it? Also, Travis seems to be failing. My internet connection is terrible, so it's not showing me why, but that has to be passing before we can merge too 😄

@Ophirr33
Copy link
Contributor Author

Ophirr33 commented May 26, 2016

I'm seeing the command "make tidy && make check-notidy -j4" exited with 2."
Does this mean I'm failing a style check in the way I formatted i?

On Thu, May 26, 2016 at 7:31 PM, Steve Klabnik notifications@github.com
wrote:

I am not seeing the fix, did you push it? Also, Travis seems to be
failing. My internet connection is terrible, so it's not showing me why,
but that has to be passing before we can merge too 😄


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#33893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/APLtwY7DcL-HwGDm5iAMTFu25n52mruoks5qFi1CgaJpZM4IoBuH
.

@Ophirr33
Copy link
Contributor Author

Trailing white space was the issue. I fixed the offending lines.

@tbu-
Copy link
Contributor

tbu- commented May 27, 2016

I don't think we should commit to this weird behavior, we should probably copy some other language's standard library here.

@GuillaumeGomez
Copy link
Member

Please squash your commits.

@Ophirr33
Copy link
Contributor Author

Rebase doesn't seem to be doing the trick. Am I squashing incorrectly? (git newbie here)

@steveklabnik
Copy link
Member

Added documentation clarifying the behavior of split when used with the empty string and contiguous separators.
@Ophirr33
Copy link
Contributor Author

Got it! Thanks again

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented May 28, 2016

📌 Commit feb0b27 has been approved by steveklabnik

@bors
Copy link
Collaborator

bors commented May 29, 2016

⌛ Testing commit feb0b27 with merge 69da70d...

@bors
Copy link
Collaborator

bors commented May 29, 2016

💔 Test failed - auto-linux-64-opt-rustbuild

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented May 29, 2016

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented May 29, 2016

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: save-temps was moved under the -C switch #33902

@bors
Copy link
Collaborator

bors commented May 29, 2016

📌 Commit feb0b27 has been approved by GuillaumeGomez

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 29, 2016
…uillaumeGomez

Added examples/docs to split in str.rs

Added documentation clarifying the behavior of split when used with the empty string and contiguous separators. Addresses issue [33882](rust-lang#33882). This is my first time contributing to rust, so forgive me if I'm skipping any of the contribution steps.
Fixes rust-lang#33882
bors added a commit that referenced this pull request May 29, 2016
Rollup of 10 pull requests

- Successful merges: #33793, #33893, #33902, #33912, #33913, #33914, #33917, #33931, #33937, #33938
- Failed merges:
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 29, 2016
…uillaumeGomez

Added examples/docs to split in str.rs

Added documentation clarifying the behavior of split when used with the empty string and contiguous separators. Addresses issue [33882](rust-lang#33882). This is my first time contributing to rust, so forgive me if I'm skipping any of the contribution steps.
Fixes rust-lang#33882
@tbu-
Copy link
Contributor

tbu- commented May 30, 2016

Maybe it is the wanted behavior. With the current results, find and split return similar things.

@GuillaumeGomez
Copy link
Member

@bors: retry

Manishearth added a commit to Manishearth/rust that referenced this pull request May 30, 2016
…uillaumeGomez

Added examples/docs to split in str.rs

Added documentation clarifying the behavior of split when used with the empty string and contiguous separators. Addresses issue [33882](rust-lang#33882). This is my first time contributing to rust, so forgive me if I'm skipping any of the contribution steps.
Fixes rust-lang#33882
bors added a commit that referenced this pull request May 30, 2016
Rollup of 8 pull requests

- Successful merges: #33793, #33893, #33912, #33913, #33914, #33917, #33937, #33938
- Failed merges:
@bors bors merged commit feb0b27 into rust-lang:master May 30, 2016
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.

7 participants