Support `merged` pull requests for `github` and `gitlab` by cardil · Pull Request #311 · psss/did

cardil · 2023-07-26T15:54:29Z

Changes

Listing merged pull requests on Github
Listing merged merge requests on Gitlab

psss · 2023-07-27T10:00:19Z

Thanks for the pull request. Actually, the PullRequestsClosed stats are intended for listing merged pull requests:

* Pull requests closed on github: 9
    * teemtee/tmt#2221 - Fix test checking custom destination for libraries
    * teemtee/tmt#2216 - Create plans to cover individual step features
    * teemtee/tmt#2206 - Group discover/fmf options, improve wording a bit
    * teemtee/tmt#2203 - Add template option to polarion report
    * teemtee/tmt#2178 - Add cache_property for things that are generated...
    * teemtee/tmt#2173 - Simplify public git conversion with a declarative list
    * teemtee/tmt#2172 - Clean up logging in `tmt.utils.create_directory()`
    * teemtee/tmt#2171 - Spec-based container becomes generic over input...
    * teemtee/tmt#2160 - Move test framework code into distinct framework...

* Pull requests reviewed on github: 6
    * teemtee/tmt#2203 - Add template option to polarion report
    * teemtee/tmt#2178 - Add cache_property for things that are generated...
    * teemtee/tmt#2173 - Simplify public git conversion with a declarative list
    * teemtee/tmt#2172 - Clean up logging in `tmt.utils.create_directory()`
    * teemtee/tmt#2171 - Spec-based container becomes generic over input...
    * teemtee/tmt#2160 - Move test framework code into distinct framework...

* Merged pull requests on github: 3
    * teemtee/tmt#2221 - Fix test checking custom destination for libraries
    * teemtee/tmt#2216 - Create plans to cover individual step features
    * teemtee/tmt#2206 - Group discover/fmf options, improve wording a bit

Not sure how exactly the query author:{0}+merged:{1}..{2} works but definitely some of the pull requests merged by me during this week are not listed. See above.

2uasimojo · 2023-11-15T23:35:34Z

I'll say that I don't understand the logic behind "closed". When I use --github-pull-requests-closed the list doesn't seem to include any pull requests I authored. What I want to see (non-exclusively) is pull requests I worked on (authored or co-authored) that have merged. If this PR makes that possible, I'm all for it.

cardil · 2023-11-16T10:24:05Z

PR can be closed because of being merged or declined. This takes just the successful PRs.

psss · 2024-01-03T13:35:43Z

I'll say that I don't understand the logic behind "closed". When I use --github-pull-requests-closed the list doesn't seem to include any pull requests I authored. What I want to see (non-exclusively) is pull requests I worked on (authored or co-authored) that have merged. If this PR makes that possible, I'm all for it.

Pull requests authored by a user are covered by pull-requests-created. The intention behind the pull-requests-closed stat was to give the credit to the person who took care of merging the pull request. This can cover making sure that tests passed and whatever action or process is agreed in the project. Such person should be set as the pull request assignee. Do you propose a different approach?

PR can be closed because of being merged or declined. This takes just the successful PRs.

Understood. It might make sense to separate closed (without merge) and merged pull requests. However, we need to make sure the stat is working as expected. As mentioned in the comment above, some of the pull requests merged by me in the given time frame are not reported by the new stat. Could you please, have a look into that?

2uasimojo · 2024-01-03T15:38:49Z

Hi @psss!

Pull requests authored by a user are covered by pull-requests-created. The intention behind the pull-requests-closed stat was to give the credit to the person who took care of merging the pull request. This can cover making sure that tests passed and whatever action or process is agreed in the project. Such person should be set as the pull request assignee. Do you propose a different approach?

I think of the assignee as the reviewer*. I dig the idea of did having a knob to report all the PRs I reviewed. Useful subsets would be:

PR is still open (my "review queue"), possibly reporting how many reviews I've already left on it.
PR was closed because merged.
PR was closed without merge.

--pull-requests-created is really great... but it would be really useful to see a similar breakdown for those, which I think was the intention behind this PR.

Having worked on CLIs for more than 20y, I appreciate that you may not want to maintain a massive proliferation of flags. That said, I could envision the following non-mutually-exclusive options that could be used in conjunction with all/most of the --github-* ones:

--open (n/a -- or just a guaranteed empty set -- for --github-*-closed)
--merged
--abandoned

...where the default is --open --merged --abandoned

I haven't thought through whether/how these could also apply to --jira-*, but it could be similar.

*I imagine different shops use different processes, so this may not apply universally, but this is how it's done at Red Hat, at least in the OpenShift org, so I know it's not just me :)

psss · 2024-01-03T16:04:01Z

I think of the assignee as the reviewer*.

Well, for reviewers there's a dedicated field, right? I would not suggest to duplicate the meaning.

PR is still open (my "review queue"), possibly reporting how many reviews I've already left on it.

Just to clarify, the existing pull-requests-reviewed stat does not cover this use case? Unfortunately, the api for reviewed pull reuqests does not always produce correct results (reported the issue several years ago but no progress on that). So I'm not sure how much this can be improved.

PR was closed because merged.

+1 for having pull-requests-merged

PR was closed without merge.

Your suggested name pull-requests-abandoned makes it less ambiguous, so +1 for this as well.

2uasimojo · 2024-01-03T20:38:38Z

I think of the assignee as the reviewer*.

Well, for reviewers there's a dedicated field, right? I would not suggest to duplicate the meaning.

Mm, fair point. The difference being that any schmo can leave a review, whereas in our process the assignee's reviews are the "important" ones -- specifically the ones that the author expects to move the PR toward getting merged. I guess the subtleties of different processes come into play here... but empirically having separate flags for "assigned" vs "reviewed" PRs would give users the flexibility to report in whatever way is most appropriate for them.

PR is still open (my "review queue"), possibly reporting how many reviews I've already left on it.

Just to clarify, the existing pull-requests-reviewed stat does not cover this use case?

If I could further filter by open/abandoned/merged, I would think so, yes.

cardil · 2024-01-08T19:22:11Z

@psss As mentioned in the #311 (comment), some of the pull requests merged by me in the given time frame are not reported by the new stat. Could you please, have a look into that?

This is straightforward. The -merged is reporting only the PRs you have authored/created, and then it was merged.

The -closed also lists the PRs authored/created by other people, you have approved. That's different.

I need the merged PR (authored by me), as I report the code I, personally, contributed.

psss · 2024-01-08T19:40:30Z

I need the merged PR (authored by me), as I report the code I, personally, contributed.

Isn't the credit for authoring a pull request covered by pull-requests-created? What is the work you want to track if you don't review and don't merge the pull request? I would say the contribution of the author is to create the pull request.

cardil · 2024-01-09T10:29:38Z

@psss Isn't the credit for authoring a pull request covered by pull-requests-created? What is the work you want to track if you don't review and don't merge the pull request? I would say the contribution of the author is to create the pull request.

The created, might not be merged. So, created contain 3 types: ongoing PRs, successful, and dropped. I just want successful ones, which I've created - merged. Also, the time is important. Creation time != merge time. I want the merge time.

psss · 2024-10-04T16:29:46Z

Thanks much for the clarification and sorry for the late response. So it's about getting the work until the finish (merging) and the date is clearly different. Makes sense now. Let's do it as you suggest. Could you please rebase on the latest main?

sandrobonazzola · 2025-05-28T16:04:50Z

Can you please rebase?

cardil · 2025-11-17T18:08:15Z

@sandrobonazzola: Can you please rebase?

Done. Sorry for being inactive on this!

sandrobonazzola

The GitHub part looks good for merging. Some questions on the GitLab one.

Other questions:

Should we filter by merge date or update date? The PR discussion suggests merge date, but the implementation seems to use update date.
Should we add unit tests similar to the existing GitLab tests?

bsipocz · 2025-11-18T11:57:04Z

In its current form this should close #371

cardil · 2025-11-18T12:07:45Z

@sandrobonazzola:

Should we filter by merge date or update date? The PR discussion suggests merge date, but the implementation seems to use update date.

That's right. I'll update the code to use the merge date. Those two dates are often the same, but the MR might be updated after it's merged, so yes.

Should we add unit tests similar to the existing GitLab tests?

Yep, I'll add those...

cardil · 2025-11-18T13:26:22Z

@cardil: I'll update the code to use the merge date.

@cardil: Yep, I'll add those...

Both fixed in dd7c2d5

cardil · 2025-11-19T12:08:23Z

The readthedocs #30370221 failure was likely due to yesterday's Github outage: https://www.githubstatus.com/incidents/5q7nmlxz30sk

Felixoid · 2026-02-26T21:50:27Z

Yes, please, it's a pretty useful stat.

psss

Looks good to me and works nice! Thanks for implementing this. Just two minor suggestions.

psss · 2026-03-24T11:02:35Z

@Felixoid, could you please have a look at the few last comments? We'll be releasing did-0.23 soon-ish. Would be good include this as well.

Felixoid · 2026-03-24T11:05:39Z

With my pleasure, but I don't have the right to apply them =)

@cardil, could you make this last spurt,

psss · 2026-03-24T11:08:53Z

Ah, yeah, sorry for mixing names, @Felixoid.
@cardil, would have some time for this?

Felixoid · 2026-03-24T21:12:26Z

I addressed the review points in f7256c9, so if @psss or @sandrobonazzola has permissions to edit the branch, the following is possible:

git fetch origin f7256c9ecbbe31c3318c62eadf0f5c086ecb8024
git push git@github.com:cardil/psss-did.git f7256c9ecbbe31c3318c62eadf0f5c086ecb8024:feature/merged-pr

Hopefully, it will allow merging it soon

cardil · 2026-03-25T11:31:27Z

I think I applied the review comments and rebased with the latest code.

I hope it's now correct!

Filter by merged_at timestamp (not updated_at) and add tests for both GitLab and GitHub merge-requests-merged functionality.

Transform MR data structure to include target_title and target_type fields expected by parent Issue class, eliminating the need for method overrides.

- Remove unnecessary iid() override in MergedRequest class - Fix 'Merge requests merged' name in gitlab stats - Fix 'Pull requests merged' name in github stats - Add available stats docs to github and gitlab plugin docstrings Assisted-by: 🤖 Claude Sonnet 4.6

- github: pull-requests-closed tracks PRs assigned to user (merged or declined) - gitlab: merge-requests-closed tracks when user pressed the Merge button Assisted-by: 🤖 Claude Sonnet 4.6

psss

Thanks much! Now looks good. Added just a few minor changes in 6da3df0.

Should be covered.

cardil · 2026-03-27T15:52:37Z

3 years. One of my longest PRs! 🎉

Thanks for merging. I really appreciate it!

psss · 2026-03-27T16:04:18Z

3 years. One of my longest PRs! 🎉

🫣 🫣 🫣

Oh... Sorry for that! And... well... congrats! 😁

Thanks for merging. I really appreciate it!

Thanks for the patience / endurance! 👍

pcahyna · 2026-04-13T09:54:06Z

+pull-requests-merged
+    pull requests authored by the user that were merged (merged_at
+    timestamp falls within the reporting period)
+


Are all these stats enabled by default? If so, how to disable some of them?

Yes, by default all stats are enabled. That applies in general for all plugins. If you need only selected stats, then you can explicitly list them on the command line, e.g.:

did --github-issues-created --github-pull-requests-created

As for now there is no better way. It could be a nice enhancement to make enabling/disabling individual stats configurable.

@psss thanks for the reply. Do I understand correctly that explicitly enabling one of them (like --github-issues-created) also disables all the others?

Yes, it's either shows all stats or only requested.

Yes, only provided stats will be shown.

psss changed the title ~~[WIP] 💝 Listing Merged PRs for Github and Gitlab~~ Support merged pull requests for github and gitlab Oct 4, 2024

psss self-assigned this Oct 4, 2024

psss added plugin | gitlab plugin | github labels Oct 4, 2024

psss added this to the 0.22 milestone Oct 4, 2024

psss removed this from the 0.22 milestone Jun 3, 2025

psss mentioned this pull request Jun 19, 2025

Add support for merged pull requests #371

Closed

cardil force-pushed the feature/merged-pr branch from 857b9f6 to 2c698a5 Compare November 17, 2025 18:02

cardil marked this pull request as ready for review November 17, 2025 18:05

sandrobonazzola reviewed Nov 18, 2025

View reviewed changes

Comment thread did/plugins/gitlab.py Outdated

Comment thread did/plugins/gitlab.py Outdated

cardil force-pushed the feature/merged-pr branch from 2c698a5 to c048fbb Compare November 18, 2025 11:30

cardil force-pushed the feature/merged-pr branch from c048fbb to dd7c2d5 Compare November 18, 2025 13:21

sandrobonazzola force-pushed the feature/merged-pr branch from 8f5e5ee to ae4b8d0 Compare November 20, 2025 11:39

kwk removed their request for review December 9, 2025 11:58

sandrobonazzola force-pushed the feature/merged-pr branch 2 times, most recently from 090cc09 to eb49e39 Compare December 19, 2025 10:49

sandrobonazzola force-pushed the feature/merged-pr branch from eb49e39 to 7f4c282 Compare February 16, 2026 07:56

sandrobonazzola previously requested changes Feb 27, 2026

View reviewed changes

Comment thread did/plugins/gitlab.py Outdated

psss requested changes Feb 27, 2026

View reviewed changes

Comment thread did/plugins/github.py

Comment thread did/plugins/gitlab.py Outdated

psss reviewed Feb 27, 2026

View reviewed changes

Comment thread did/plugins/github.py Outdated

psss added this to the 0.23 milestone Feb 27, 2026

Felixoid reviewed Mar 25, 2026

View reviewed changes

Comment thread did/plugins/github.py Outdated

cardil and others added 6 commits March 27, 2026 13:59

Listing Merged PRs for Github and Gitlab

8b7a4d5

Fix merge date filtering and add unit tests

f760d30

Filter by merged_at timestamp (not updated_at) and add tests for both GitLab and GitHub merge-requests-merged functionality.

Fix markdown formatting for merged requests

30004cd

Transform MR data structure to include target_title and target_type fields expected by parent Issue class, eliminating the need for method overrides.

Fix stats docs: clarify closed vs merged semantics

8f8f45f

- github: pull-requests-closed tracks PRs assigned to user (merged or declined) - gitlab: merge-requests-closed tracks when user pressed the Merge button Assisted-by: 🤖 Claude Sonnet 4.6

Use definition list for overview, fix test

6da3df0

psss force-pushed the feature/merged-pr branch from ed93779 to 6da3df0 Compare March 27, 2026 13:01

psss approved these changes Mar 27, 2026

View reviewed changes

psss requested a review from sandrobonazzola March 27, 2026 13:04

psss merged commit f13232b into psss:main Mar 27, 2026
15 checks passed

psss self-assigned this Mar 27, 2026

pcahyna reviewed Apr 13, 2026

View reviewed changes

Conversation

cardil commented Jul 26, 2023 • edited by psss Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

psss commented Jul 27, 2023

Uh oh!

2uasimojo commented Nov 15, 2023

Uh oh!

cardil commented Nov 16, 2023

Uh oh!

psss commented Jan 3, 2024

Uh oh!

2uasimojo commented Jan 3, 2024

Uh oh!

psss commented Jan 3, 2024

Uh oh!

2uasimojo commented Jan 3, 2024

Uh oh!

cardil commented Jan 8, 2024

Uh oh!

psss commented Jan 8, 2024

Uh oh!

cardil commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psss commented Oct 4, 2024

Uh oh!

sandrobonazzola commented May 28, 2025

Uh oh!

cardil commented Nov 17, 2025

Uh oh!

sandrobonazzola left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bsipocz commented Nov 18, 2025

Uh oh!

cardil commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cardil commented Nov 18, 2025

Uh oh!

cardil commented Nov 19, 2025

Uh oh!

Felixoid commented Feb 26, 2026

Uh oh!

Uh oh!

psss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

psss commented Mar 24, 2026

Uh oh!

Felixoid commented Mar 24, 2026

Uh oh!

psss commented Mar 24, 2026

Uh oh!

Felixoid commented Mar 24, 2026

Uh oh!

Uh oh!

cardil commented Mar 25, 2026

Uh oh!

psss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cardil commented Mar 27, 2026

Uh oh!

psss commented Mar 27, 2026

Uh oh!

pcahyna Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

psss Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

cardil commented Jul 26, 2023 •

edited by psss

Loading

cardil commented Jan 9, 2024 •

edited

Loading

cardil commented Nov 18, 2025 •

edited

Loading