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

Fix cast string into number #1607

Closed
wants to merge 1 commit into from
Closed

Fix cast string into number #1607

wants to merge 1 commit into from

Conversation

Kikobeats
Copy link

No description provided.

@algobot
Copy link
Contributor

algobot commented Nov 25, 2016

By analyzing the blame information on this pull request, we identified @bobylito to be a potential reviewer

@vvo
Copy link
Contributor

vvo commented Nov 28, 2016

Nice @Kikobeats! What was the underlying bug here (and the effects?), can you provide a way to reproduce it so we can write a bug fix?

@Kikobeats
Copy link
Author

Yes, basically, when it try to calculate the new state, because the number is serialized the sum is wrong:

p=0
p=01
p=011
p=0111

expected (and the thing that this issue resolve)

p=0
p=1
p=2
p=3

@vvo
Copy link
Contributor

vvo commented Nov 28, 2016

I think I get it, since the state can be serialized in the URL, when you read it back p is now a string even if it contains a number?

@Kikobeats
Copy link
Author

exactly

@vvo
Copy link
Contributor

vvo commented Nov 28, 2016

Are you willing to add a test for this? Otherwise I'll do it.

@Kikobeats
Copy link
Author

Better if you do it and I learn for the next time

@vvo
Copy link
Contributor

vvo commented Nov 29, 2016

moved to #1619

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.

3 participants