search

nimiq / nimiq-utils

Simple helper libraries for Nimiq frontends
Apache License 2.0
6 stars 14 forks source link

NumberFormattingUtils #28

Closed danimoh closed 5 years ago

danimoh commented 5 years ago

I changed the implementation to your class based approach with some changes, please have a look. I also switched to exporting the whole class instead of single helper methods. I exported single helper methods before for better tree-shakeability. But now with the class based approach I prefer exporting the whole class as it is likely not well tree-shaekable anymore anyways. I will adapt the use in vue-components and the hub tomorrow.

I also added an equals method which can be useful for your requested approximation check.

sisou commented 5 years ago

On mobile it doesn't, and on desktop it only shows for the last recent force push, as far as I know.

Individual commits while working on it are better, and then consolidate them all together when actually merging.

I thought you preferred that too?

danimoh commented 5 years ago

On mobile it doesn't, and on desktop it only shows for the last recent force push, as far as I know.

I'm not sure about its behavior on repeated force pushs. It might also diff between the state before the first force push and the state after the last one?

Individual commits while working on it are better, and then consolidate them all together when actually merging. I thought you preferred that too?

When a requested change only results in a small change in a previous commit, I often prefer to rebase.