-
Notifications
You must be signed in to change notification settings - Fork 45
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
"Number of pull requests" data appears inaccurate/misleading #32
Comments
Hi, I can take a look on October 13th the earliest, I'll be on KubeCon next week. |
Actually I've checked charts and we are considering PRs/Issues activities there not just opened PRs, and this is consistent even if we take data from non-github projects (we then count activitie son bugs/emials etc), now cc @caniszczyk what to do:
This needs decision: either (1) or (2). |
This still needs decision, so I'll keep this open. |
Hi @lukaszgryglicki, thanks for looking into this and propose potential options. My vote is 2 as it is what number of PRs really means :) |
OK but this needs an approval and new reports will now look differentb than they were, let's wait for a decision. |
/cc @caniszczyk who I assume is the decider |
In the velocity reports, we report "The y-axis is the total number of pull requests and issues".
From the query, this is determined by the total amount of
PullRequestEvent
s: https://github.com/cncf/velocity/blob/8e1d1c189b65e2544fae7aec43c6381f9e4b4d82/BigQuery/velocity_cncf.sql#L19C18-L19C34.A
PullRequestEvent
does not correlate 1:1 with "a PR" in a way a person would interpret a count of PRs, in my opinion. There are two reasonable approaches (merged PRs or opened PRs, strongly preferring merged PRs), neither of which this counts.Per docs 'The action that was performed. Can be one of opened, edited, closed, reopened, assigned, unassigned, review_requested, review_request_removed, labeled, unlabeled, and synchronize.'. However, in practice I found this doesn't seem to be the case. Looking at a single day across github:
Even without the other possible events, we at least appear to be double counting PRs?
The text was updated successfully, but these errors were encountered: