Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Masun dev#2041

Closed
maggiemsft wants to merge 3 commits into
dotnet:masterfrom
maggiemsft:masun-dev
Closed

Masun dev#2041
maggiemsft wants to merge 3 commits into
dotnet:masterfrom
maggiemsft:masun-dev

Conversation

@maggiemsft
Copy link
Copy Markdown

This change implements string trim char and try to solve issue discussed in https://github.com/dotnet/coreclr/issues/1826
Appreciate comments and review
This is also the bootcamp assignment
cc: @ellismg @sneshaf @Anniepoh

@dnfclas
Copy link
Copy Markdown

dnfclas commented Nov 13, 2015

Hi @maggiemsft, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link
Copy Markdown

dnfclas commented Nov 13, 2015

@maggiemsft, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

Comment thread src/mscorlib/src/System/String.cs Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should the parameter be named trimChar to be consistent with the existing trimChars?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed. Should I submit a new change and PR for it?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can just create a new commit (or amend the existing one) and then push the new changes to GitHub. You don't need to create a new PR.

@justinvp
Copy link
Copy Markdown

@ellismg, if this PR can go in coreclr master, can we also go ahead and merge #895 in coreclr master? Then submit a separate PR against corefx future to actually expose the APIs in the reference assembly?

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Nov 17, 2015

Dup of #2070

@jkotas jkotas closed this Nov 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants