Feature: Feedback - clarify "contribution" in repo pages contributions / lottery factor

[jpmcb]

Suggested solution

This is some feedback I'm surfacing here that I brought up with the team earlier today: the phrasing of "Contribution" can be abit confusing depending on the audience and the context. Primarily, in repo pages:

These "Contributions" and contributions in the "Lottery factor" chart are what we've defined as any pull request opened by someone. Example, in the wesbos repo above, these are not contributions that have been accepted, but rather, have simply been opened:

This repo hasn't had many merged contirbutions, so this may be a confusing snapshot of this repo for people who don't know how we count these.

Another thought: this has the potential for people to game the system and spam repos with junk pull requests that get closed immediately but still get counted in our contributions chart and lottery factor chart.

---

I'm not certain the action here: I would like others thoughts here too.

We might want to think about the lottery factor only looking at "accepted" / merged PRs. We likely also need to educate our users on how we think about "Contributions" since OpenSauced is pretty forgiving on what we would count as a contribution. Where others may have much stricter understandings of what a contribution is (was it accepted? are they a member of the org? did they sign the CLA? etc. etc.)

[github-actions]

Thanks for the issue, our team will look into it as soon as possible! If you would like to work on this issue, please wait for us to decide if it's ready. The issue will be ready to work on once we remove the "needs triage" label.

To claim an issue that does not have the "needs triage" label, please leave a comment that says ".take". If you have any questions, please reach out to us on Discord or follow up on the issue itself.

For full info on how to contribute, please check out our contributors guide.

[BekahHW]

@jpmcb I think we definitely need some clarification here. We have talked about contributions as "more than green squares" for a while, so I do think it's important that we're very specific here to not mix messaging. If the lottery factor is only looking at merged PRs, then that's what we should specify. And I do think in this case, it makes sense. For instance, I would generally say that writing an issue is a contribution, but that doesn't decrease the lottery factor. The PR that gets merged does. A project can have 5 great issues and no one working on them so the lottery factor would be high. Merged PRs makes the most sense in defining this, IMO.

[jpmcb]

I would generally say that writing an issue is a contribution, but that doesn't decrease the lottery factor

Yeah, that is a really, really excellent point and I agree.

In the API, this is a pretty small change:

diff --git a/src/timescale/pull_request_github_events.service.ts b/src/timescale/pull_request_github_events.service.ts
index bd137eb6..9bbaa984 100644
--- a/src/timescale/pull_request_github_events.service.ts
+++ b/src/timescale/pull_request_github_events.service.ts
@@ -264,7 +266,8 @@ export class PullRequestGithubEventsService {
       .addSelect("count(*)", "count")
       .where(`'${startDate}'::TIMESTAMP >= "pull_request_github_events"."event_time"`)
       .andWhere(`'${startDate}'::TIMESTAMP - INTERVAL '${range} days' <= "pull_request_github_events"."event_time"`)
-      .andWhere(`"pull_request_github_events"."pr_action" = 'opened'`)
+      .andWhere(`"pull_request_github_events"."pr_action" = 'closed'`)
+      .andWhere(`"pull_request_github_events"."pr_is_merged" = TRUE`)
       .andWhere(`LOWER("pull_request_github_events"."repo_name") IN (:...repoNames)`, {
         repoNames,
       })

where we only get PRs for the lottery factor that have action = "closed" and pr_is_merged = true. Playing around with this locally on my client, this is what it would surface for the wesbos repo:

which seems much more accurate to the reality of the project, we would just need to figure out the messaging.

[jpmcb]

Chatted with @bdougie on this - we're abit too close to launching this to change it now. But yes, in the future, with more and more context on other devstats, we can revisit this and tweak it.