Skip to content
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

Display minutes for time #388

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

LeoRosse
Copy link

pull request for issue #387

Hi everyone, I implemented the features I needed, as I described in the issue.

@dan-kez
Copy link

dan-kez commented May 13, 2020

This would be excellent to have merged if possible - just implemented my own (less good) solution here #398.

Having this would really expand the ability to use this library in my opinion.

@davidgoli is there any chance we could have this merged?

Comment on lines +142 to +143
AT()
COLON()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main draw back with this approach is that lists of times are not supported though that may be intentional.

For example in the list case you would have to list all permutation for this to work

// Invalid
every day at 9:00 and 11:30 => RRULE:FREQ=DAILY;BYHOUR=9,11;BYMINUTE=0,30
// valid
every day at 9:00, 9:30, 11:00 and 11:30 => RRULE:FREQ=DAILY;BYHOUR=9,11;BYMINUTE=0,30

Personally, I think it's reasonable to limit this to one hour time combo to prevent confusion

.npmrc Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@acro5piano
Copy link

Any updates?

@davidgoli
Copy link
Collaborator

This diff is too large. Please revert all lint-style formatting changes (semicolons, quotes around property names, reformatting etc), rebase on master, and re-push. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants