Skip to content

Conversation

rkilingr
Copy link
Contributor

@tidwall
Copy link
Owner

tidwall commented Jan 31, 2021

I can't tell if this change fixes anything. It looks unnecessary. Does it address a specific problem?

@rkilingr
Copy link
Contributor Author

I can't tell if this change fixes anything. It looks unnecessary. Does it address a specific problem?

No it's just for documentation, doesn't fix anything specific, Not a priority. Let me know if I can make any changes to make it clear, Just thought it would be useful

@rkilingr
Copy link
Contributor Author

@tidwall
Let me know if I can make any changes, otherwise will close this if not necessary. Thanks!

@tidwall
Copy link
Owner

tidwall commented Feb 13, 2021

I would prefer to keep the numbers hardcoded. Adding constants are not needed imo.

But I don't mind if you added a comment above the line

if f < -9007199254740991 || f > 9007199254740991 {

that includes the links to the https://tc39.es/ecma262 page.

@rkilingr
Copy link
Contributor Author

@tidwall Thanks, have made the change

@tidwall tidwall merged commit b977acb into tidwall:master Feb 15, 2021
@tidwall
Copy link
Owner

tidwall commented Feb 15, 2021

👍 Looks good.

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.

2 participants