-
-
Notifications
You must be signed in to change notification settings - Fork 405
Conversation
AI disclosure: I used OpenAI Codex (GPT-5.6) to identify uncovered code paths, explain the relevant project code, review the user-authored tests, and verify formatting, tests, and coverage.
This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments.
Profile summary:
GitHub user: karurukaruru
🟡 Some concerns found with user's profile.
🟢 No concerns found with recent PR activity.
🟢 No concerns found with recent issue activity.
For a more detailed report, run `gh-profiler karurukaruru`.
Full profile
GitHub user: karurukaruru
🟡 Some concerns found with user's profile.
🟢 Account age: 4 days
🟡 Profile information:
name: カルル
twitter: https://x.com/djkaruru
Empty fields: company, blog, location, email, bio
🟢 No concerns found with recent PR activity.
1 PR opened in the last 21 days.
0 opened against repos the user owns.
0 opened against repos in publicly associated orgs.
1 opened against external repos.
🟢 0 of 1 external PRs merged in the last 21 days.
🟢 0 of 1 external PRs closed without merging in the last 21 days.
🟢 No concerns found with recent issue activity.
🟢 No new issues opened in the last 21 days.
This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments.
Documentation build overview
📚 icalendar | 🛠️ Build #33973084 | 📁 Comparing 89756ff against latest (f8d109c)
45 files changed · + 1 added · ± 44 modified
+ Added
± Modified
404.htmlgenindex.htmlcontribute/code-of-conduct.htmlcontribute/development.htmlhow-to/parse-errors.html_modules/icalendar/alarms.html_modules/icalendar/error.htmlcontribute/documentation/style-guide.htmlreference/api/icalendar.cal.component.htmlreference/api/icalendar.cal.event.html- and 34 more...
karurukaruru
commented
Aug 3, 2026
Hello, thank you for reviewing my first PR.
I created this GitHub account four days ago. The bot may have marked my PR because this account is new.
I am not a native English speaker. I started learning Python again about one month ago. I used OpenAI Codex (GPT-5.6) to help me understand the project, review the tests, write English, and read the test results.
I added tests for five places that did not have tests before. I did not change the project code. I only added tests. I checked the changes and I can explain them.
Could you please review my PR? Thank you very much.
@stevepiercy
stevepiercy
left a comment
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.
Thank you for your first contribution! The work LGTM.
Let's get a technical review. There may be a preferred way to organize the tests in existing test files or parametrize them.
@SashankBhamidi
SashankBhamidi
left a comment
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.
Thanks for the contribution, and welcome. I ran all five tests and they pass.
One thing to address inline.
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.
The tests should be split into their natural homes rather than collected in a new file.
test_vunknown_reprbelongs insrc/icalendar/tests/test_issue_1445.py, that's where allvUnknowntests live.test_vcategory_hashandtest_vcategory_reprbelong insrc/icalendar/tests/prop/test_unit.pyalongside the existingvCategorytests at line 335.test_vweekday_rejects_invalid_jcal_valuebelongs insrc/icalendar/tests/prop/test_vWeekday.py.test_vdate_rejects_invalid_calendar_datebelongs insrc/icalendar/tests/prop/test_date_and_time.py.
Once you've moved them, this new file can be deleted.
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.
Thank you very much for the detailed guidance. I have moved each test to the existing test file you suggested and removed src/icalendar/tests/prop/test_issue_698_more_coverage.py. I also ran the five affected tests and the Ruff lint and formatting checks, and they pass. I appreciate your help in showing me the project's preferred test organization.
AI disclosure: I used OpenAI Codex (GPT-5.6) to apply the requested test organization changes, review the diff, and verify formatting and the five affected tests.
@niccokunzmann
niccokunzmann
left a comment
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.
Thanks for your PR. I will wait for Sashank to have a look again.
Uh oh!
There was an error while loading. Please reload this page.
Linked issue
Description
Adds focused tests for five uncovered paths involving unknown-property representation, category hashing and representation, invalid calendar dates, and invalid jCal weekday values. No production code is changed.
I used OpenAI Codex (GPT-5.6) to help identify uncovered paths, understand the relevant code, review the tests, and verify formatting, tests, and coverage.
Checklist