BigInteger.toArray - Githubissues

peterolson / BigInteger.js

An arbitrary length integer library for Javascript

The Unlicense

1.12k stars 187 forks source link

BigInteger.toArray #125

Closed matthewleon closed 6 years ago

matthewleon commented 6 years ago

addresses https://github.com/peterolson/BigInteger.js/issues/123

This code uses Array.fill, which breaks compatibility with legacy browsers. If this is undesirable, I can modify the code to maintain compatibility, probably at the cost of some performance.

peterolson commented 6 years ago

Hey, thanks, it looks great, you seem to have found your way around the project well. I just have two suggestions before I merge it in:

The part you added to README.md documentation shows toArray returning strings (it looks like it's copied from the toString documentation)
- As you mentioned, Array.fill breaks compatibility with legacy environments (and it requires changing to ChromeHeadless instead of PhantomJS, which seems to be breaking the Travis CI build for some reason). I'd prefer to keep compatibility here.

Thanks!

matthewleon commented 6 years ago

Okay, I believe this to be ready for review again. Thanks for your comments.

coveralls commented 6 years ago

Coverage decreased (-0.1%) to 99.405% when pulling 71834a7ff398dd8815b3ca942abc831f99970f53 on matthewleon:toArray into b7e949acdc534cc77e511a707b16f0c5e2ccfe7c on peterolson:master.

coveralls commented 6 years ago

Coverage increased (+0.005%) to 99.524% when pulling e9cad47110a8d1ef7718ac450784a9dd7f5bfd46 on matthewleon:toArray into b7e949acdc534cc77e511a707b16f0c5e2ccfe7c on peterolson:master.

matthewleon commented 6 years ago

I'll try to please coveralls.

matthewleon commented 6 years ago

Okay, we're good with coveralls :). Things should be truly reviewable now.

peterolson commented 6 years ago

Looks good to me, thanks!