-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for "ly" #8
Comments
@fega , @simison : Hi. Can you provide a sample usecase which would be solved by adding 'ly' support to human interval? |
@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? #19 |
@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 ? |
That sounds good to me 👍 |
There's plenty to do on Agenda side as well so feel free to just dive in. :-) |
@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. |
I meant new one as it indeed seems easier. 👍
…On Sun, 22 Mar 2020, 11:04 Sampath Kumar Krishnan, ***@***.***> 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
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAVJAD4QAFF5U5XRRK24ETRIXIDHANCNFSM4DRM27VQ>
.
|
like "monthly"
"weekly"
etc :)
The text was updated successfully, but these errors were encountered: