Skip to content

Conversation

@dmpots
Copy link
Contributor

@dmpots dmpots commented Jun 27, 2025

This PR updates the links used to show the PR contribution stats of the user requesting commit access to the LLVM project. Previously we would only show the PRs that were currently opened by the user because the /pulls/<username> endpoint automatically applies the is:open filter.

The contribution guidelines suggest that the user should have at least 3 merged PRs to be considered for commit access so this seems like a relevant data point to add.

We now show all PRs that a user has created (not just the open ones) and a separate link for the PRs that are merged.

This PR updates the links used to show the PR contribution stats of the user
requesting commit access to the LLVM project. The link previously would only
show the PRs that were currently opened by the user because the
`/pulls/<username>` endpoint automatically applies the `is:open` filter.

The contribution guidelines suggest that the user should have at least 3 merged
PRs to be considered for commit access so this seems like a relavant data point
to add.

We now show all PRs that the user has created and a separate link for the PRs
that are merged.
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

LGTM, but I agree with Jlalond@ comment. Please address that before landing this change.

Also, please let tstellar@ review this change before landing as he is the one who goes through these.

@dmpots
Copy link
Contributor Author

dmpots commented Jul 7, 2025

Also, please let tstellar@ review this change before landing as he is the one who goes through these.
@tstellar please take a look when you get a chance, thanks!

@dmpots
Copy link
Contributor Author

dmpots commented Jul 21, 2025

Also, please let tstellar@ review this change before landing as he is the one who goes through these.

@tstellar please take a look when you get a chance, thanks!

@dmpots
Copy link
Contributor Author

dmpots commented Jul 28, 2025

Also, please let tstellar@ review this change before landing as he is the one who goes through these.

@tstellar please take a look when you get a chance, thanks!

@dmpots
Copy link
Contributor Author

dmpots commented Sep 23, 2025

Also, please let tstellar@ review this change before landing as he is the one who goes through these.

@tstellar please take a look when you get a chance, thanks!

@tstellar
Copy link
Collaborator

@dmpots Can we add is:merged to the query? That would be the most helpful, those are what actually count towards the requirements.

@dmpots
Copy link
Contributor Author

dmpots commented Sep 24, 2025

Can we add is:merged to the query? That would be the most helpful, those are what actually count towards the requirements.

@tstellar do you mean that we should only have the one link that shows the merged PRs and not the separate link for total vs. merged?

It is currently adding the is:merged as a separate link

merged_prs_url = total_prs_url + "+is%3Amerged"

Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

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

I see what you are saying now, what you have here is fine.

@dmpots dmpots merged commit c924e7a into llvm:main Sep 25, 2025
7 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
This PR updates the links used to show the PR contribution stats of the
user requesting commit access to the LLVM project. Previously we would
only show the PRs that were currently opened by the user because the
`/pulls/<username>` endpoint automatically applies the `is:open` filter.

The contribution guidelines suggest that the user should have at least 3
merged PRs to be considered for commit access so this seems like a
relevant data point to add.

We now show all PRs that a user has created (not just the open ones) and
a separate link for the PRs that are merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants