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

round to six decimal places #53

Merged
merged 3 commits into from Nov 16, 2022
Merged

round to six decimal places #53

merged 3 commits into from Nov 16, 2022

Conversation

jgravois
Copy link
Collaborator

@jgravois jgravois commented Nov 5, 2022

alternative to #52

  • round instead of truncate, for slightly more accurate results
  • yanked const/let in favor of good ol 'var'
    this codebase is old as hell. its due for a facelift, but i'd rather let (groan!) sleeping dogs lie for now.
  • added WKT tests to confirm those values are rounded too
  • updated doc

does this seem reasonable @KD33?

ps: i would have just pushed to the PR that was already open, but you either unticked the box or the option was disabled because you're PRing directly from your own main branch

Copy link

@KD33 KD33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks a lot!

@jgravois
Copy link
Collaborator Author

jgravois commented Nov 8, 2022

@springmeyer in #49 i suggested you make main a protected branch without realizing that one of the default rules prohibits non-admins like me from merging PRs without code review 🤦‍♂️

at this point you have two options:

  1. untick the box below if you'd like me to be able to take small stuff like this coast to coast on my own
  2. if you'd rather I not go rogue, you can continue to approve PRs individually

i'm happy either way, and will seek feedback regardless if i ever decide to pitch something substantive.

Screen Shot 2022-11-07 at 4 13 15 PM

@springmeyer
Copy link
Owner

@jgravois I've unchecked the box requiring approvals to make sure you are not blocked from merging small stuff. Thanks!

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.

None yet

3 participants