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

Initialize stamped and ended, implementing the small optimization todo #347

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

yuwash
Copy link
Contributor

@yuwash yuwash commented Nov 26, 2023

The lack of initialization of the stamped variable was causing a failure from an unexpected place when running it with a faulty ical that lacked a BEGIN:V… before having the first DTSTAMP. Probably the same with ended. Only affects bad strings, so it’s more of an error handling improvement…

@tobixen
Copy link
Member

tobixen commented Nov 27, 2023

Thanks for the contribution. Unfortunately I don't have time to do a code review right now, but possibly later today or tomorrow :-)

@yuwash yuwash force-pushed the initialize-stamped branch from c436d65 to d9e485c Compare November 27, 2023 16:31
@tobixen
Copy link
Member

tobixen commented Nov 28, 2023

It would be nice with ...

  1. test code proving that the current code is breaking

  2. unit test proving that the class does what it's intended to do

@tobixen
Copy link
Member

tobixen commented Nov 28, 2023

And 3 also, updates to the CHANGELOG.md-file. Anyway, if this actually solves a bug, I suppose it's best to just merge it now and indicate in a separate issue that we need tests.

@tobixen tobixen added this pull request to the merge queue Nov 28, 2023
Merged via the queue into python-caldav:master with commit 242c4c0 Nov 28, 2023
7 checks passed
tobixen added a commit that referenced this pull request Nov 28, 2023
yuwash added a commit to yuwash/caldav that referenced this pull request Nov 28, 2023
yuwash added a commit to yuwash/caldav that referenced this pull request Nov 28, 2023
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