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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature format number #16

Closed
wants to merge 4 commits into from
Closed

Conversation

flexd
Copy link
Contributor

@flexd flexd commented Jan 16, 2015

This has been useful in a number of projects and I think go-humanize is a suitable home.

See examples here http://play.golang.org/p/LXc1Ddm1lJ
Much thanks to @gorhill for original code 馃憤

@dustin
Copy link
Owner

dustin commented Jan 17, 2015

Could use a bit more test coverage if possible.

I also smashed these two changes together as to produce one logical change with tests.

@flexd
Copy link
Contributor Author

flexd commented Feb 4, 2015

What more test coverage would you like? I added a few more.

@dustin
Copy link
Owner

dustin commented Feb 4, 2015

Well, I'd like to not reduce test coverage when adding new features. There's currently no code in go-humanize that isn't covered by tests. Last time I looked, this added code that wasn't covered, so it's either dead code, or bugs left to the user to discover. :)

@flexd
Copy link
Contributor Author

flexd commented Feb 4, 2015

Hm, well unless code inside FormatFloat is counted and I have missed some possible inputs/outcomes, FormatInteger is just a wrapper around FormatFloat. I could add identical tests for that function as well.

@dustin
Copy link
Owner

dustin commented Feb 4, 2015

go test -coverprofile=c.out && go tool cover -html=c.out

@flexd
Copy link
Contributor Author

flexd commented Feb 4, 2015

Ah I see, I will fix that.

@dustin
Copy link
Owner

dustin commented Apr 13, 2015

Done, thanks!

@dustin dustin closed this Apr 13, 2015
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

2 participants