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

Fixed binding issue where was binding to the in the let expression rat... #7

Merged
merged 3 commits into from Nov 19, 2014

Conversation

Projects
None yet
2 participants
@jcollard
Contributor

jcollard commented Nov 19, 2014

...her than the parameter.

b was being shadowed by another binding and was causing the range to be broken. Renaming a and b to l and h fixes this.

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 19, 2014

Contributor

Also, fixed a range issue with floats.

Contributor

jcollard commented Nov 19, 2014

Also, fixed a range issue with floats.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Nov 19, 2014

Member

Is it also plausible to rename b to base and keep a and b the same? I think that'd be my preferred route.

A lot of the names in this code were really crazy and I did not feel comfortable making them longer, or really touching them at all, because I did not have an intuition of what they meant.

Member

evancz commented Nov 19, 2014

Is it also plausible to rename b to base and keep a and b the same? I think that'd be my preferred route.

A lot of the names in this code were really crazy and I did not feel comfortable making them longer, or really touching them at all, because I did not have an intuition of what they meant.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Nov 19, 2014

Member

Reasoning for a and b is that I want to express going in that these are numbers with no ordering assumed, and I feel like l and h are sort of non-standard number names and suggest an invariant that's not true.

Member

evancz commented Nov 19, 2014

Reasoning for a and b is that I want to express going in that these are numbers with no ordering assumed, and I feel like l and h are sort of non-standard number names and suggest an invariant that's not true.

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 19, 2014

Contributor

Yeah, changing b to base should work.

I actually didn't realize let was letrec not let* which lead to a lot of confusion debugging this problem. :-P

Contributor

jcollard commented Nov 19, 2014

Yeah, changing b to base should work.

I actually didn't realize let was letrec not let* which lead to a lot of confusion debugging this problem. :-P

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 19, 2014

Contributor

fwiw, I agree that a and b are better names.

Contributor

jcollard commented Nov 19, 2014

fwiw, I agree that a and b are better names.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Nov 19, 2014

Member

Awesome, looks great! Thanks for sorting this out, and sorry for introducing these mistakes :/

Member

evancz commented Nov 19, 2014

Awesome, looks great! Thanks for sorting this out, and sorry for introducing these mistakes :/

evancz pushed a commit that referenced this pull request Nov 19, 2014

Merge pull request #7 from jcollard/random-lo-hi-bindings
Fixed binding issue where  was binding to the  in the let expression rat...

@evancz evancz merged commit 9a2c219 into elm:master Nov 19, 2014

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 19, 2014

Contributor

No problem. I am just glad we caught them before 0.14.

Contributor

jcollard commented Nov 19, 2014

No problem. I am just glad we caught them before 0.14.

@jcollard jcollard deleted the jcollard:random-lo-hi-bindings branch Nov 19, 2014

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Nov 19, 2014

Member

One benefit of 0.14 is that we can release core independent of the compiler now. This one could have been 1.0.1 :)

Member

evancz commented Nov 19, 2014

One benefit of 0.14 is that we can release core independent of the compiler now. This one could have been 1.0.1 :)

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