search

jvm-profiling-tools / honest-profiler

A sampling JVM profiler without the safepoint sample bias
https://github.com/RichardWarburton/honest-profiler/wiki
MIT License
1.25k stars 146 forks source link

Fix : Improve readability #212

Open PhRX opened 7 years ago

PhRX commented 7 years ago

Improved readability by adding options for thousands separators, choosing decimal separators etc.

Replaces deleted PR #194 .

PhRX commented 7 years ago

Commit 58dd0f5 reverts all previous commits from 13de748 up to and including 1e1e31e, so please ignore those commits in the PR commit list.

RichardWarburton commented 7 years ago

This is still a very large PR - could it be split up somehow?

PhRX commented 7 years ago

@RichardWarburton I reviewed the PR, but much like #210 I don't see any way to prise apart the changes.

As in #210 there's only one logical split I can see - separating out the formatting logic (i.e. all methods for printing a number in a particular format) from the configuration logic (selecting the actual format). But since the second obviously depends on the first, again I do not see how I can create 2 independent branches out of this.

Any advice would be greatly appreciated.

RichardWarburton commented 7 years ago

I think splitting out the formatting logic and configuration logic changes would be a good idea. I mean this is still a 1700 line diff PR in it's current form.

Btw - my apologies for the delays in getting back to you on these PRs. I've found it very difficult to allocate time to this project until recently.

PhRX commented 7 years ago

I have absolutely no headroom for looking at this at the moment, I'll try and get to it ASAP.