jamesplease / moment-business

Utilities for work days in Moment. (Western workweeks only.)
MIT License
100 stars 27 forks source link

Add float precision flag. #56

Closed jreeter closed 5 years ago

jreeter commented 7 years ago

Adds floating flag to return higher precision week day counts.

jamesplease commented 7 years ago

Thanks @jreeter ! Would you mind adding new tests to verify that this produces the expected results? The “hole” utilities that I wrote only have tests assuming integers, so I’m a little bit wary of whether they can handle non-integer inputs.

jreeter commented 7 years ago

Yea, I'll see about adding tests and documentation to the API. Seem to be getting some weird test error when running npm run test:

[14:52:30] 'test' errored after 30 ms
[14:52:30] AssertionError: true === false
    at Def.Dp.bases (/Users/josh/dev/moment-business/node_modules/ast-types/lib/types.js:408:12)
    at Object.<anonymous> (/Users/josh/dev/moment-business/node_modules/6to5/lib/6to5/patch.js:18:4)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at Object.<anonymous> (/Users/josh/dev/moment-business/node_modules/6to5/lib/6to5/util.js:3:1)
    at Module._compile (module.js:413:34)

I'll try to look into this as well.

jamesplease commented 7 years ago

Thanks. This library is very old, so CI is building it on Node 0.10. If you're using a node version manager, the simplest approach may just be developing on Node 0.10. If you separately want to update the build system to support a newer Node, by all means, go for it! But I'd prefer if we do that separately from adding this new functionality in.

Thanks for taking the time to improve this library!

jamesplease commented 5 years ago

I'm going to close this PR due to inactivity. If there's still interest in this feature, just leave me a note and we can definitely continue the conversation.