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

Fix vector tests to work on non en-US culture machines #31

Merged
merged 1 commit into from Nov 12, 2014

Conversation

Projects
None yet
@adamralph
Contributor

adamralph commented Nov 12, 2014

This fixes the following test failure (on my de-CH machine)

image

After the change the build succeeds.

@luisrudge

This comment has been minimized.

Show comment
Hide comment

👍

@joshfree

This comment has been minimized.

Show comment
Hide comment
@joshfree

joshfree Nov 12, 2014

Member

LGTM. running this through our CI

Member

joshfree commented Nov 12, 2014

LGTM. running this through our CI

@MaximRouiller

This comment has been minimized.

Show comment
Hide comment
@MaximRouiller

MaximRouiller Nov 12, 2014

Is that officially the first community pull request? If yes, will that be the first ACCEPTED community pull request? 👍

Is that officially the first community pull request? If yes, will that be the first ACCEPTED community pull request? 👍

@nguerrera

This comment has been minimized.

Show comment
Hide comment
Member

nguerrera commented Nov 12, 2014

@gaearon

This comment has been minimized.

Show comment
Hide comment

gaearon commented Nov 12, 2014

👍

@FiveTimesTheFun

This comment has been minimized.

Show comment
Hide comment
@FiveTimesTheFun

FiveTimesTheFun Nov 12, 2014

Contributor

Awesome - thank you!

Contributor

FiveTimesTheFun commented Nov 12, 2014

Awesome - thank you!

@mellinoe

This comment has been minimized.

Show comment
Hide comment
@mellinoe

mellinoe Nov 12, 2014

Contributor

Great catch, thanks for submitting this. I believe there is an analogous test in Vector4Tests.cs that needs to be changed, as well (it also tests this same case for Vector4).

👍

Contributor

mellinoe commented Nov 12, 2014

Great catch, thanks for submitting this. I believe there is an analogous test in Vector4Tests.cs that needs to be changed, as well (it also tests this same case for Vector4).

👍

@damianh

This comment has been minimized.

Show comment
Hide comment
@damianh

damianh Nov 12, 2014

@adamralph I was hoping this would be a "Remove all regions" troll PR. Much disappoint.

damianh commented Nov 12, 2014

@adamralph I was hoping this would be a "Remove all regions" troll PR. Much disappoint.

@dotnet-bot dotnet-bot merged commit 5ec20d0 into dotnet:master Nov 12, 2014

@adamralph adamralph deleted the adamralph:fix-tests branch Nov 12, 2014

@davepermen

This comment has been minimized.

Show comment
Hide comment
@davepermen

davepermen Nov 12, 2014

👍 congratulations

👍 congratulations

@pgermishuys

This comment has been minimized.

Show comment
Hide comment

congrats!

@shanselman

This comment has been minimized.

Show comment
Hide comment

Woot

@alfhenrik

This comment has been minimized.

Show comment
Hide comment
@alfhenrik

alfhenrik Nov 12, 2014

Awesome dude!

Awesome dude!

@Stephanvs

This comment has been minimized.

Show comment
Hide comment
@Stephanvs

Stephanvs Nov 12, 2014

Haha awesome! First Accepted PR 👍

Haha awesome! First Accepted PR 👍

@adamralph

This comment has been minimized.

Show comment
Hide comment
@adamralph

adamralph Nov 12, 2014

Contributor

@mellinoe that test was using the wrong variable for assertion which is why it was passing. I've fixed it in another PR #32

Contributor

adamralph commented Nov 12, 2014

@mellinoe that test was using the wrong variable for assertion which is why it was passing. I've fixed it in another PR #32

@DamianEdwards

This comment has been minimized.

Show comment
Hide comment
@jeffdoolittle

This comment has been minimized.

Show comment
Hide comment

👍

@paulcbetts

This comment has been minimized.

Show comment
Hide comment

Fuck yeah @adamralph

@NickMOrlando

This comment has been minimized.

Show comment
Hide comment

👍

@Mpdreamz

This comment has been minimized.

Show comment
Hide comment
@Mpdreamz

Mpdreamz Nov 12, 2014

Collaborator

💥 Immortalised yourself here @adamralph 👍

Collaborator

Mpdreamz commented Nov 12, 2014

💥 Immortalised yourself here @adamralph 👍

@juanfranblanco

This comment has been minimized.

Show comment
Hide comment
@juanfranblanco

juanfranblanco Nov 12, 2014

Awesome you are the first

Awesome you are the first

@charleslbryant

This comment has been minimized.

Show comment
Hide comment

Sweet

@tiagobabo

This comment has been minimized.

Show comment
Hide comment

👍

@tugberkugurlu

This comment has been minimized.

Show comment
Hide comment

💥 🎆

@ciriarte

This comment has been minimized.

Show comment
Hide comment
@ciriarte

ciriarte Nov 12, 2014

👍 fantastic!

👍 fantastic!

@erinpagemd

This comment has been minimized.

Show comment
Hide comment

👍

@AlexZeitler

This comment has been minimized.

Show comment
Hide comment
@AlexZeitler

AlexZeitler Nov 12, 2014

Holy shit - it happened 🙈🙏

Holy shit - it happened 🙈🙏

@Haacked

This comment has been minimized.

Show comment
Hide comment

Haacked commented Nov 12, 2014

selfie-0

@dennisdoomen

This comment has been minimized.

Show comment
Hide comment
@dennisdoomen

dennisdoomen Nov 12, 2014

Congrats @adamralph. Let's drink on it next week!

Congrats @adamralph. Let's drink on it next week!

@adamralph

This comment has been minimized.

Show comment
Hide comment
@adamralph

adamralph Nov 12, 2014

Contributor

@dennisdoomen looking forward to that!

Contributor

adamralph commented Nov 12, 2014

@dennisdoomen looking forward to that!

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Nov 12, 2014

Yes. Yes. Yes.

dsyme commented Nov 12, 2014

Yes. Yes. Yes.

@MaximRouiller

This comment has been minimized.

Show comment
Hide comment
@MaximRouiller

MaximRouiller Nov 12, 2014

On my god.... it was the first community PR accepted, built, unit tested and stamped by Microsoft!

😮

On my god.... it was the first community PR accepted, built, unit tested and stamped by Microsoft!

😮

@keithelder

This comment has been minimized.

Show comment
Hide comment
@keithelder

keithelder Nov 12, 2014

That didn't take long!

That didn't take long!

@plaurin

This comment has been minimized.

Show comment
Hide comment
@plaurin

plaurin Nov 12, 2014

This is amazing! OSS works!

plaurin commented Nov 12, 2014

This is amazing! OSS works!

@caioproiete

This comment has been minimized.

Show comment
Hide comment
@caioproiete

caioproiete Nov 12, 2014

This is HUGE!! Awesome!

This is HUGE!! Awesome!

@sikanderiqbal

This comment has been minimized.

Show comment
Hide comment

History!

@petervandenhout

This comment has been minimized.

Show comment
Hide comment
@shiftkey

This comment has been minimized.

Show comment
Hide comment
@shiftkey

shiftkey Nov 12, 2014

Obligatory:

Obligatory:

@giggio

This comment has been minimized.

Show comment
Hide comment
@giggio

giggio Nov 12, 2014

Just awesome!

giggio commented Nov 12, 2014

Just awesome!

@DavidChristiansen

This comment has been minimized.

Show comment
Hide comment
@DavidChristiansen

DavidChristiansen Nov 12, 2014

Woot @adamralph - You the man!! That's at least two beers for you next week! (@ buildstuff.lt)

Woot @adamralph - You the man!! That's at least two beers for you next week! (@ buildstuff.lt)

@glennblock

This comment has been minimized.

Show comment
Hide comment

Nice job @adamralph!

@khellang

This comment has been minimized.

Show comment
Hide comment
@khellang

khellang Nov 12, 2014

Collaborator

Yay! 😁👍

Yay

Collaborator

khellang commented Nov 12, 2014

Yay! 😁👍

Yay

@andrewconnell

This comment has been minimized.

Show comment
Hide comment
@andrewconnell

andrewconnell Nov 13, 2014

No freaking way... give that man a Surface Pro 3 & XBOX One!

xhwl9dq

No more...

No freaking way... give that man a Surface Pro 3 & XBOX One!

xhwl9dq

No more...

@SharePointing

This comment has been minimized.

Show comment
Hide comment
@LoungeFlyZ

This comment has been minimized.

Show comment
Hide comment
@VesaJuvonen

This comment has been minimized.

Show comment
Hide comment
@georgeracu

This comment has been minimized.

Show comment
Hide comment

👍

@jbuiss0n

This comment has been minimized.

Show comment
Hide comment

Awesome !

@rylancates

This comment has been minimized.

Show comment
Hide comment
@rylancates

rylancates Nov 13, 2014

So great 👍

So great 👍

@Zimmergren

This comment has been minimized.

Show comment
Hide comment

Nicely done @adamralph :-)

@ezechibritton

This comment has been minimized.

Show comment
Hide comment
@ezechibritton

ezechibritton Nov 15, 2014

@adamralph Simply awesome, you just earned your spot in history buddy. Can I name drop and say @adamralph was my team mate! :-)

@adamralph Simply awesome, you just earned your spot in history buddy. Can I name drop and say @adamralph was my team mate! :-)

@odiszapc

This comment has been minimized.

Show comment
Hide comment
@odiszapc

odiszapc Nov 20, 2014

Fuck yeah? Fuck yeahhhh!!!!!!!!!!11

Fuck yeah? Fuck yeahhhh!!!!!!!!!!11

@gy

This comment has been minimized.

Show comment
Hide comment
@gy

gy Nov 21, 2014

Awesome @adamralph 💯

gy commented Nov 21, 2014

Awesome @adamralph 💯

@manigandham

This comment has been minimized.

Show comment
Hide comment
@manigandham

manigandham Nov 21, 2014

Great to see Microsoft embracing open-source completely, this is fantastic! Great job getting things started @adamralph!

Great to see Microsoft embracing open-source completely, this is fantastic! Great job getting things started @adamralph!

@kunjee17

This comment has been minimized.

Show comment
Hide comment
@kunjee17

kunjee17 Nov 21, 2014

@adamralph man, you did it... Thanks and Thanks to team dot net for all this OSS work. +1

@adamralph man, you did it... Thanks and Thanks to team dot net for all this OSS work. +1

@phillip-haydon

This comment has been minimized.

Show comment
Hide comment
@phillip-haydon

phillip-haydon Nov 21, 2014

image

image

image

Never thought I would see the day.

image

image

image

Never thought I would see the day.

@henriksen

This comment has been minimized.

Show comment
Hide comment
@henriksen

henriksen Nov 22, 2014

Nice! Well done!

Nice! Well done!

@vidyavardhan

This comment has been minimized.

Show comment
Hide comment

👍

@fagnercarvalho

This comment has been minimized.

Show comment
Hide comment
@fagnercarvalho

fagnercarvalho Nov 23, 2014

This is historical! 😃

This is historical! 😃

@cyberid41

This comment has been minimized.

Show comment
Hide comment
@cyberid41

cyberid41 Dec 1, 2014

Congratulations 👍

Congratulations 👍

@AnthonyDGreen

This comment has been minimized.

Show comment
Hide comment
@AnthonyDGreen

AnthonyDGreen Jun 18, 2015

Couldn't resist, LGTM!
#NDCOslo

Couldn't resist, LGTM!
#NDCOslo

pgavlin added a commit to pgavlin/corefx that referenced this pull request Sep 22, 2015

Merge pull request #31 from pgavlin/SocketsCleanup
Clean up TCP{Client,Listener}.

sepidehkh added a commit to sepidehkh/corefx that referenced this pull request Feb 3, 2016

@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment