search

agenda / human-interval

Human readable time distances for javascript
Other
459 stars 13 forks source link

Add support for "ly" #8

Open fega opened 7 years ago

fega commented 7 years ago

like "monthly" "weekly" etc :)

sampathBlam commented 4 years ago

@fega , @simison : Hi. Can you provide a sample usecase which would be solved by adding 'ly' support to human interval? With respect to agenda, I could not think of any, as agenda already has .every and similar interval based time configurations for scheduling tasks. I understand that agenda is not the only package that depends on human-interval. If we could get a valid usecase, and the expectations for this 'ly' support, it would be great and I can start working on a PR right away for that.

simison commented 4 years ago

@sampathBlam I don't think it's important and I wouldn't complicate this code for it.

Would you like to pick up this refactor tho? https://github.com/agenda/human-interval/pull/19

sampathBlam commented 4 years ago

@simison : Does it make sense if i fork/pull from their repo (with their current set of refactorings), resolve the merge conflicts, add more tests if needed, verify and create a new PR?

Or should I first create a PR for their repo on the same branch, thereby, if the PR was accepted in their repo, it would automatically be reflected in #19 ?

simison commented 4 years ago

That sounds good to me 👍

simison commented 4 years ago

There's plenty to do on Agenda side as well so feel free to just dive in. :-)

sampathBlam commented 4 years ago

@simison : Sorry i did not get you. Should I create a new PR or try to update the current one ? The second one might be a slow process.

simison commented 4 years ago

I meant new one as it indeed seems easier. 👍

On Sun, 22 Mar 2020, 11:04 Sampath Kumar Krishnan, notifications@github.com wrote:

@simison https://github.com/simison : Sorry i did not get you. Should I create a new PR or try to update the current one ? The second one might be a slow process.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/agenda/human-interval/issues/8#issuecomment-602168272, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVJAD4QAFF5U5XRRK24ETRIXIDHANCNFSM4DRM27VQ .

sampathBlam commented 4 years ago

@simison : Done. Created a new PR #33