fix: Fix time_in_draft iterator exhaustion and misnamed event #610
+58
−21
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: zkoppert <6935431+zkoppert@users.noreply.github.com>
Copilot
AI
changed the title
Copilot finished work on behalf of
zkoppert
October 3, 2025 18:15
zkoppert
changed the title
- Change event name from converted_to_draft to convert_to_draft to match actual GitHub timeline events - Remove problematic datetime parsing that was causing type errors - Update time calculation logic to properly handle draft state transitions - Make issue_metrics.py executable - Update corresponding test cases to use correct event names This fixes the draft PR metrics functionality to work with the actual GitHub API events. Signed-off-by: Zack Koppert <zkoppert@github.com>
zkoppert
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes bugs in the draft time measurement functionality by correcting the GitHub API event name and improving compatibility with iterator-based event sources. The changes ensure the code works correctly with the real GitHub API.
- Standardized event name from "converted_to_draft" to "convert_to_draft" throughout code and tests
- Fixed iterator exhaustion issue by converting events to a list upfront
- Added test coverage for iterator-based events and improved test data consistency
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| time_in_draft.py | Updated event name, fixed iterator handling, and added result rounding |
| test_time_in_draft.py | Updated all test cases with correct event name and added iterator test |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zack Koppert <zkoppert@github.com>
zkoppert
approved these changes
Oct 4, 2025
jmeridth
approved these changes
Oct 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request updates the logic and tests for measuring the time a pull request spends in draft state. The main changes include standardizing the event name for draft conversions, improving compatibility with iterator-based event sources, updating test data types, and rounding the output for consistency.
Bug fixes and compatibility improvements:
Standardized the event name from
"converted_to_draft"to"convert_to_draft"throughout both the implementation and all test cases, ensuring consistency with the real GitHub API event names. [1] [2] [3] [4] [5] [6] [7] [8] [9]Modified the
measure_time_in_draftfunction to convert the events iterator to a list at the start, ensuring compatibility with the real GitHub API, which returns an iterator that can only be consumed once.Test enhancements:
Added a new test (
test_time_in_draft_with_iterator_events) to verify thatmeasure_time_in_draftworks correctly when the events method returns an iterator, simulating real-world API behavior.Updated test setup to use
datetimeobjects forcreated_atinstead of ISO-formatted strings, aligning with the rest of the code and improving clarity. [1] [2]Accuracy improvements:
The result of
measure_time_in_draftis now rounded to the nearest second for improved accuracy and consistency.Updated variable names in the implementation for clarity (e.g.,
converted_to_draft_events→convert_to_draft_events). [1] [2]These changes improve reliability, maintainability, and correctness of the draft time measurement logic and its associated tests. fixes #608