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

fix(cache): cache key comparison ignores milliseconds #490

Merged
merged 3 commits into from
Nov 28, 2021

Conversation

hgrsd
Copy link
Contributor

@hgrsd hgrsd commented Nov 14, 2021

See #489. The comparison used for cache keys ignores milliseconds, leading to false positive cache hits.

In this PR I've changed the comparison strategy to compare the unix timestamps of the relevant dates instead. Let me know whether you think this is the right approach :)

Thanks for contributing to rrule!

To submit a pull request, please verify that you have done the following:

  • Merged in or rebased on the latest master commit
  • Linked to an existing bug or issue describing the bug or feature you're
    addressing
  • Written one or more tests showing that your change works as advertised

@hgrsd
Copy link
Contributor Author

hgrsd commented Nov 14, 2021

Hello @jakubroztocil @lyschoening @davidgoli 👋 I'd be grateful for a review and your view on the proposed change :) Thanks in advance!

@hgrsd
Copy link
Contributor Author

hgrsd commented Nov 28, 2021

@jakubroztocil @lyschoening @davidgoli just checking in to see if you have had any time to take a look at this? Let me know!

@jkbrzt jkbrzt merged commit f446250 into jkbrzt:master Nov 28, 2021
@jkbrzt
Copy link
Owner

jkbrzt commented Nov 28, 2021

Thanks!

@hgrsd
Copy link
Contributor Author

hgrsd commented Nov 28, 2021

Thanks for merging! 😃 Do you have a sense of when this might be released? Anything I can do to help do that?

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.

2 participants