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

devstats,repo_groups: generate repo_groups.sql #332

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

dhiller
Copy link
Contributor

@dhiller dhiller commented Oct 7, 2024

What this PR does / why we need it:

CNCF devstats project provides dashboards for KubeVirt that show GitHub data 2. This PR is a required part to keep the repository groups of the user statistics by repository 3 in sync with the content of sigs.yaml. PR kubevirt/project-infra#3701 will change the job to use this tool.

This PR adds a go tool that generates SQL statements to update repository groups for CNCF devstats 1.

See #330 for more details.

Tested in job https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/logs/postsubmit-community-update-kubevirt-devstats-repo-sql/1848991035489783808
which produced changes reflected from PR #334 in a PR against repo_groups.sql for KubeVirt: https://github.com/cncf/devstats/pull/80/files

Background

KubeVirt repository groups are updated with an SQL script 4 - we want these repository groups to be updated automatically whenever there's changes in our sigs.yaml.

A while ago we had decided to directly align our repository groups to the ownership references inside the sig definitions - in effect this maps any change in a repository to one or more SIGs.

When calling the tool, i.e.

go run ./robots/cmd/devstats/...

it generates SQL code that is targeted to update the kubevirt repo_groups

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

@kubevirt-bot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Oct 7, 2024
@dhiller dhiller force-pushed the update-devstats-repo-groups branch from 0b86fd4 to 67fd05c Compare October 7, 2024 15:48
@dhiller dhiller marked this pull request as ready for review October 7, 2024 16:05
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 7, 2024
@dhiller dhiller force-pushed the update-devstats-repo-groups branch from 67fd05c to a7e1bac Compare October 21, 2024 16:24
@dhiller
Copy link
Contributor Author

dhiller commented Oct 21, 2024

Updated PR to generate like this, from comment to cncf/devstats#77, where that PR was initially created from a previous version which contained SQL syntax errors, i.e. comma missing etc.

@dhiller
Copy link
Contributor Author

dhiller commented Oct 21, 2024

@xpivarc @jean-edouard may I ask for a review?

@dhiller dhiller force-pushed the update-devstats-repo-groups branch from a7e1bac to 09dcd18 Compare October 23, 2024 07:31
@dhiller
Copy link
Contributor Author

dhiller commented Oct 23, 2024

@vladikr
Copy link
Member

vladikr commented Oct 23, 2024

@dhiller if there a way to see the end result? how is this going to look like?

@dhiller
Copy link
Contributor Author

dhiller commented Oct 23, 2024

@dhiller if there a way to see the end result? how is this going to look like?

I just tested this code with a prow job that I created manually - this was the resulting PR that was created: cncf/devstats#80

@vladikr
Copy link
Member

vladikr commented Oct 29, 2024

/approve
Thanks, I saw the generated out put. took me awhile to wrap my head around this, but I'm fine with this.

@kubevirt-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vladikr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2024
@aburdenthehand
Copy link
Member

@dhiller Forgive my ignorance but does this PR enable us to look at stats at a repository level, rather than a repo group level? (I ask because I was trying to look for devstats on activity in the community repo and couldn't see a way to do so.)

@dhiller
Copy link
Contributor Author

dhiller commented Oct 30, 2024

@dhiller Forgive my ignorance but does this PR enable us to look at stats at a repository level, rather than a repo group level? (I ask because I was trying to look for devstats on activity in the community repo and couldn't see a way to do so.)

TBH this is a very good question, which I don't know the answer to out of my head. I always thought of the repo_group conceptually as something that would collect activity of i.e. a group like a sig and gather all activity over several repositories together - so I thought individual repo activity would be obvious by looking at the contributors metrics here: https://github.com/kubevirt/community/graphs/contributors

But yes - naturally it would make sense to look at devstats to find something, and I bet there is, i just would need to find it 😄

@aburdenthehand
Copy link
Member

The contributor metrics is useful, but only represents commits (and amount of code changes), whereas devstats provides a more complete picture with reviews, comments, etc
But don't let this line of questioning get in the way of this PR moving forward :)

@dhiller
Copy link
Contributor Author

dhiller commented Oct 30, 2024

/uncc @jobbler

@kubevirt-bot kubevirt-bot removed the request for review from jobbler October 30, 2024 14:23
@dhiller
Copy link
Contributor Author

dhiller commented Oct 30, 2024

The contributor metrics is useful, but only represents commits (and amount of code changes), whereas devstats provides a more complete picture with reviews, comments, etc But don't let this line of questioning get in the way of this PR moving forward :)

Thinking about this - since we are defining our own custom groups anyway we can configure them anyway we like (at least that's my assumption, would need to let Lukas Griglycki from CNCF devstats project confirm that).

When comparing the k8s devstats vs kubevirt devstats I saw that the k8s devstats has something we could need also:
https://k8s.devstats.cncf.io/d/49/github-stats-by-repository?orgId=1

I've asked Lukasz whether we can get a copy of that dashboard.

@dhiller
Copy link
Contributor Author

dhiller commented Oct 30, 2024

The contributor metrics is useful, but only represents commits (and amount of code changes), whereas devstats provides a more complete picture with reviews, comments, etc But don't let this line of questioning get in the way of this PR moving forward :)

Thinking about this - since we are defining our own custom groups anyway we can configure them anyway we like (at least that's my assumption, would need to let Lukas Griglycki from CNCF devstats project confirm that).

When comparing the k8s devstats vs kubevirt devstats I saw that the k8s devstats has something we could need also: k8s.devstats.cncf.io/d/49/github-stats-by-repository?orgId=1

I've asked Lukasz whether we can get a copy of that dashboard.

Didn't look to closely - these wouldn't help us either. So I think the best approach is to define another set of repo-groups - one per repo, so we can select activity by user per repo via a i.e. k/community repo group.

@aburdenthehand WDYT?

@aburdenthehand
Copy link
Member

@dhiller Maaaybe it's a stretch of the 'repo-group' term but if that's something we can do relatively easily then that would be really useful.

@dhiller
Copy link
Contributor Author

dhiller commented Nov 4, 2024

@dhiller Maaaybe it's a stretch of the 'repo-group' term but if that's something we can do relatively easily then that would be really useful.

@aburdenthehand if you don't mind - I'd want to do this in a follow-up PR then 😊
IOW looking for an lgtm from you here :)

@aburdenthehand
Copy link
Member

/lgtm
Not at all :)

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2024
@kubevirt-bot kubevirt-bot merged commit b6db995 into kubevirt:main Nov 4, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants