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 range with resource_ids. #171

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

jabiertxof
Copy link
Contributor

Fixes #75

@niccokunzmann
Copy link
Owner

niccokunzmann commented Sep 2, 2024

Hi, could you create a Pr against the issue-75 branch? that one has the tests in it that need passing.

Looking at the code, we cannot replace the core but somehow need to use one of the modifications when it is time.

@niccokunzmann niccokunzmann merged commit 2bce830 into niccokunzmann:main Sep 9, 2024
5 checks passed
@niccokunzmann
Copy link
Owner

Thanks! I merge it and check ...

@jabiertxof
Copy link
Contributor Author

jabiertxof commented Sep 9, 2024

Hi Nicco, I'm not familiar with github, sorry for don't reply at time. Seems my code is removed from master, but current master code not handle the issue, could you explain to me why you remove it? Thanks so much for your work.

Regards, Jabier.

@jabiertxof
Copy link
Contributor Author

jabiertxof commented Sep 10, 2024 via email

@niccokunzmann
Copy link
Owner

Hello, thanks for coming back to this! I removed the code from the master branch after merge because of

This project's development is driven by tests. - README

The code did not fix the tests however it was a good place to put it.
What it did is though that the event with the RANGE parameter would also modify the events before it.
I merged it for these reasons:

  1. I value your contribution
  2. I wanted to check if it fixes the tests
  3. I rather want to give it a go than criticize around

I then removed it again because it did not solve the issue.

@jabiertxof
Copy link
Contributor Author

jabiertxof commented Sep 10, 2024 via email

@niccokunzmann
Copy link
Owner

Hi, let's discuss this in the issue, so it is all together in one place.

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.

bug: RANGE parameter of recurring events not recognized
2 participants