-
Notifications
You must be signed in to change notification settings - Fork 82
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
Make today/tomorrow/yesterday at the beginning of the day #14
base: master
Are you sure you want to change the base?
Conversation
We sero out hour/minutes/seconds
Issue #13 is related. |
👍 I think this makes sense as well, if you have something like "yesterday at 5pm", you'll still get the seconds of Maybe an option to always zero anything that isn't specified? |
Well, it seems to be a case for plugging those rules you would like to use for a particular case. Example: import (
github.com/olebedev/when
github.com/olebedev/when/rules/en
github.com/olebedev/when/rules/common
customENGRules github.com/demisto/when/rules/en // from the above
)
w := when.New(nil)
// this rule will match before the rest ones, the order is matters
w.Add(customENGRules.CasualDate)
// ... add whatever rules you want, like:
// w.Add(en.All...)
// w.Add(common.All...) @tj, I think an option here is an unnecessarily in favor of using rules as building blocks for a case. Wdyt? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aviadl, I am happy to accept the rule you suggested but as an alternative rule which is not included in the default instance. Maybe a separate directory would be fine, like: rules/en/misc
. And the README
or Godoc with a description of the rule and examples, etc.
Co-Authored-By: Oleg Lebedev <[email protected]>
Not sure why the original one is valid, but i will let you take it from here, and do the changes you believe in |
Custom rules sounds reasonable to me, I didn't look at how they're implemented yet but I'll take a look! |
Hello @aviadl, Thank you for your contribution. I appreciate the effort you put into this matter. After reconsideration, I agree with the change you suggested for "valid." However, I have one concern remaining. Users who are already utilizing the library may encounter unexpected behavior when they update it. I'm considering whether it would be beneficial to introduce versioning to the library. I would like to hear your thoughts on this matter. Related #16. Best regards, |
IMO i would treat the current behaviour as a bug, so i think that this fix is fine, and not need for versioning BTW - you can start working with git releases, and then it will change only for customers that take the latest release |
zero out hour/minutes/seconds when working with 'today/tomorrow/yesterday'