-
Notifications
You must be signed in to change notification settings - Fork 159
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
Convert round calls to math.Round #777
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @icholy)
tests/math.c, line 406 at r1 (raw file):
is_nan(round(NAN)); is_eq(round(INFINITY), INFINITY); is_eq(round(-INFINITY), -INFINITY);
You cannot use equality tests with infinities, you can us this instead:
is_inf(round(INFINITY), 1);
is_inf(round(-INFINITY), -1);
Docs here: https://github.com/elliotchance/c2go/blob/master/tests/tests.h#L205-L224
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @icholy)
tests/math.c, line 406 at r1 (raw file):
Previously, elliotchance (Elliot Chance) wrote…
You cannot use equality tests with infinities, you can us this instead:
is_inf(round(INFINITY), 1); is_inf(round(-INFINITY), -1);Docs here: https://github.com/elliotchance/c2go/blob/master/tests/tests.h#L205-L224
Derp, that makes perfect sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @elliotchance)
tests/math.c, line 406 at r1 (raw file):
Previously, icholy (Ilia Choly) wrote…
Derp, that makes perfect sense.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @icholy)
tests/math.c, line 14 at r1 (raw file):
int main() { plan(359);
You will need to update this to 372
. This is an extra safety net to make sure all the tests do actually run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @elliotchance)
tests/math.c, line 14 at r1 (raw file):
Previously, elliotchance (Elliot Chance) wrote…
You will need to update this to
372
. This is an extra safety net to make sure all the tests do actually run.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build is still failing, do the tests pass locally on your machine?
Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved
I'm having issues with the
INFINITY
tests.This change is