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

Feature: Ignore PR time spent as draft, measure PRs by merge time #111

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

PressXtoChris
Copy link
Contributor

What's the feature?

Excludes the time a pull request was in draft mode when calculating how long it was open for, and uses the time a PR was merged as the time the issue was closed.

How does it work?

  • If an issue is a pull request, grab when it was marked as Ready for Review
    • To measure this, it grabs the first 50 events and looks for one called "Ready for Review".
    • If it can't find it, subsequent steps will assume the PR was never a draft and will just use the Created time
  • measure_time_to_first_response now excludes comments/reviews made before the PR was marked as Ready for Review
  • Instead of calling measure_time_to_close, PRs now use measure_time_to_merge which will key off of the Ready for Review time (if one was found)

If the user would like to exclude active drafts from the search results as well, they can add draft:false to their search query

This should resolve #37

@zkoppert zkoppert added the enhancement New feature or request label Sep 8, 2023
Copy link
Member

@zkoppert zkoppert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @PressXtoChris ! Thanks for taking the time to put this together and with all the testing!!

@zkoppert
Copy link
Member

zkoppert commented Sep 8, 2023

@PressXtoChris Do you see a good place in the README to note that time to close for a PR is measured by the time to merge excluding time as a draft?

@PressXtoChris
Copy link
Contributor Author

I marked the relevant metrics in the ReadMe with an asterisk and said they exclude draft time. IMO that section makes the most sense since it's the bit that actually talks about the metrics, while the rest is implementation details. Happy for you to move it somewhere else if you think it makes more sense though

@zkoppert zkoppert merged commit bcdaf83 into github:main Sep 11, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Measure time to merge for pull requests
2 participants