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

Add script to perform summary of merged PRs and closed issues #4125

Merged
merged 12 commits into from May 26, 2023

Conversation

registerrier
Copy link
Contributor

@registerrier registerrier commented Oct 5, 2022

Description

This pull request proposes a script to query the GitHub list of closed PRs and issues to list all merged PRs and count closed issues with a given milestone.

Usage:

python dev/github_summary.py  merged_PR 1.0 --token XXXX
python dev/github_summary.py  closed_issues 0.20 --token XXXX --print_issues True

It requires to install PyGithub (https://github.com/PyGithub/PyGithub)
Dear reviewer

@adonath adonath added this to the 1.0 milestone Oct 5, 2022
@adonath
Copy link
Member

adonath commented Oct 5, 2022

@registerrier Be default I included the dev scripts in the CI check as well, just because they don't change often. However I'm not sure the dev scripts are included in the pre-commit hook. Maybe you can check and adapt the PR...

@codecov
Copy link

codecov bot commented Oct 5, 2022

Codecov Report

Merging #4125 (8793690) into main (7a4a6f0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #4125   +/-   ##
=======================================
  Coverage   94.96%   94.96%           
=======================================
  Files         221      221           
  Lines       31445    31445           
=======================================
  Hits        29862    29862           
  Misses       1583     1583           
Impacted Files Coverage Δ
gammapy/maps/core.py 85.82% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@registerrier
Copy link
Contributor Author

@registerrier Be default I included the dev scripts in the CI check as well, just because they don't change often. However I'm not sure the dev scripts are included in the pre-commit hook. Maybe you can check and adapt the PR...

It is included in the pre-commit hooks. I was not executing them :).

@bkhelifi
Copy link
Member

bkhelifi commented Oct 6, 2022

Thanks @registerrier, great implementation.
I have two thoughts:

  • the functionality provided by the click function list_merged_PRs is the same than the one used to build a contributor list. I am wondering if it should not be into a dedicated function into a dev/utils.py... such that one can re-use it
  • this nice implementation with PyGitHub provides the same type of information than git shortlog --merges --since XX --until YY. Do I mistaken? If yes, what are the pros and cons of each method?

@adonath adonath added the backport-v1.0.x on-merge: backport to v1.0.x label Oct 7, 2022
@registerrier registerrier removed the backport-v1.0.x on-merge: backport to v1.0.x label Oct 11, 2022
@registerrier registerrier modified the milestones: 1.0, 1.1 Oct 11, 2022
@adonath
Copy link
Member

adonath commented Apr 28, 2023

It would be good to have something like this, but we should try to improve the implementation.

@registerrier registerrier added the backport-v1.1.x on-merge: backport to v1.1.x label May 24, 2023
…given milestone

Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Régis Terrier <rterrier@apc.in2p3.fr>
@registerrier
Copy link
Contributor Author

registerrier commented May 25, 2023

The code has been completely changed. It now consists in a a general class GitHubInfoExtractor which deals with the github interface (login, token, repo) and whose main method build a summary table of PRs.
This table is then parsed by a helper function to produce an output list of PRs, the list of contributors (for now based on the list of people who opened the PRs).

It is called this way:

  • to create the table
python dev/github_summary.py  create_pull_request_table --token XX  --number_min 3900 --overwrite True 
  • to create the summary output
python dev/github_summary.py  merged_PR table_pr.ecsv 1.1 v1.0.1 v1.0.2

adonath
adonath previously approved these changes May 25, 2023
Copy link
Member

@adonath adonath left a comment

Choose a reason for hiding this comment

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

Thanks a lot @registerrier, I have no comments! This is a very useful and cleanly written script!

AtreyeeS
AtreyeeS previously approved these changes May 26, 2023
Copy link
Member

@AtreyeeS AtreyeeS left a comment

Choose a reason for hiding this comment

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

Thanks @registerrier !

return result

def extract_pull_requests_table(
self, state="closed", number_min=0, include_backports=False
Copy link
Member

Choose a reason for hiding this comment

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

Github counts from 1, so I guess the default number_min = 1

dev/github_summary.py Outdated Show resolved Hide resolved
@registerrier registerrier dismissed stale reviews from AtreyeeS and adonath via d84ee87 May 26, 2023 13:49
@registerrier registerrier merged commit 8420761 into gammapy:main May 26, 2023
13 of 14 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/gammapy that referenced this pull request May 26, 2023
registerrier added a commit that referenced this pull request May 26, 2023
…5-on-v1.1.x

Backport PR #4125 on branch v1.1.x (Add script to perform summary of merged PRs and closed issues )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v1.1.x on-merge: backport to v1.1.x infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants