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

Rename std.string.splitterLines to std.string.lineSplitter #3018

Merged
merged 1 commit into from
Feb 22, 2015

Conversation

schuetzm
Copy link
Contributor

As discussed in PR #2987.

@schuetzm
Copy link
Contributor Author

Actually I'd prefer to call it byLine in analogy with std.stdio.byLine, but I'm worried about the small difference in behaviour with respect to line endings (thanks to @ntrel for pointing it out).

@ntrel
Copy link
Contributor

ntrel commented Feb 21, 2015

LGTM

1 similar comment
@quickfur
Copy link
Member

LGTM

@quickfur
Copy link
Member

ping @andralex @AndrejMitrovic

@ghost
Copy link

ghost commented Feb 21, 2015

Sorry but there's some issue with the autotester.

Sorry, it looks like you don't have permission to this project at github.

@quickfur
Copy link
Member

Brad needs to approve him before his PRs will get auto-tested.

@ghost
Copy link

ghost commented Feb 22, 2015

No, I have the same problem with Kenji's pull. :/

@quickfur
Copy link
Member

Oh? Is something wrong with the auto-tester? I did notice this past week that auto-merged PRs get credited to Brad no matter who toggled it. Wonder if that's related...

@burner
Copy link
Member

burner commented Feb 22, 2015

LGTM

@burner
Copy link
Member

burner commented Feb 22, 2015

Auto-merge toggled on

braddr added a commit that referenced this pull request Feb 22, 2015
Rename std.string.splitterLines to std.string.lineSplitter
@braddr braddr merged commit 694a827 into dlang:master Feb 22, 2015
kuettler added a commit to kuettler/phobos that referenced this pull request Feb 23, 2015
@kuettler
Copy link
Contributor

Call me old-fashioned. (And I know I am late to the party.) This is a very recent addition and I think it rather rude to rename a library function without consent of its author, especially if the author is @WalterBright. Am I really so far off?

@schuetzm
Copy link
Contributor Author

Well, there seemed to have been consensus, and Walter didn't speak up in a week so far (neither here nor in his PR). I guess he's ok with it. The important thing IMO is to get a decision before the release, which is due soon.

@schuetzm schuetzm deleted the linesplitter branch February 23, 2015 13:23
@tom-tan
Copy link
Contributor

tom-tan commented Feb 25, 2015

Was splitterLines completely deleted?
IMHO it will be better to leave splitterLines with deprecated attribute to show the migration path to the users.

@ntrel
Copy link
Contributor

ntrel commented Feb 25, 2015

@tom-tan splitterLines was never released, so it's OK to rename it.

@ntrel
Copy link
Contributor

ntrel commented Feb 25, 2015

Actually it does appear in the release branch here - we need to get this pull merged into 2.067.

@burner
Copy link
Member

burner commented Feb 25, 2015

@MartinNowak "we need to get this pull merged into 2.067."

@yebblies yebblies added this to the 2.067 milestone Feb 25, 2015
@tom-tan
Copy link
Contributor

tom-tan commented Feb 25, 2015

splitterLines was never released, so it's OK to rename it.

I see. I did not notice that.

braddr added a commit that referenced this pull request Feb 27, 2015
Rename std.string.splitterLines to std.string.lineSplitter
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.

8 participants