devstats.archive icon indicating copy to clipboard operation
devstats.archive copied to clipboard

[feature request][kubernetes] Add wg/sig label filter to Contributions chart

Open mrbobbytables opened this issue 3 years ago • 11 comments

This is a follow up from #289, #293, and #295

Adding a filter for sig/wg labels to the Contributions chart will allow SIGS and WGs that work out of shared repos to look at their metrics. With that change it should resolve all our current asks and give sig leads, subproject owners and companies the info they need 👍

mrbobbytables avatar Jun 15 '21 14:06 mrbobbytables

Will evaluate as soon as I can, currently a bit busy.

lukaszgryglicki avatar Jun 15 '21 14:06 lukaszgryglicki

So the first problem is that filter for sig/wg labels - this means it can only be applied to issues/PR, not for everything - only issue/PR objects have label(s).

lukaszgryglicki avatar Jun 21 '21 05:06 lukaszgryglicki

Is this OK for you? This will find a SIG for the following types of metrics:

  • Approves
  • Reviews
  • Comments
  • Issue comments
  • Issues
  • PRs
  • Merged PRs
  • Review comments

This will not find SIG for:

  • Commit comments
  • Commits
  • GitHub events (partially, this will count issues PRs, issues/PRs comments, not commits)
  • GitHub pushes
  • Contributions (partially, it will count issues, PRs, reviews, issue/PR comments, will not count commits). @mrbobbytables?

lukaszgryglicki avatar Jun 21 '21 05:06 lukaszgryglicki

I can have one more option - trying to connect commit event (which has commit SHA) with a PR (by using either PR head SHA or PR merge commit SHA), that way I can try to correlate PR with commit... but there is no other dashboard doing so, and, actually, I'm not 100% sure if that kind of connection is correct?

That would mean that a commit belongs to a particular SIG when:

  • There is a PR that is based on that commit (created from that commit or merged by that commit)
  • That PR's issue (in GitHub PR is also an issue, part of its data is stored on the issue object )has a corresponding SIG's sig/xyz label. Makes sense?

lukaszgryglicki avatar Jun 21 '21 05:06 lukaszgryglicki

let me know if I should continue with that approach, which means

  • Issue belongs to SIG if it has a SIG/xyz label.
  • PR belongs to SIG if its issue has SIG/xyz label.
  • Approval belongs to SIG if it's an approval of PR described above.
  • Review belongs to SIG if it's an approve of PR described above.
  • Comment belongs to a SIG if it's a comment on the issue/PR described above.
  • Commit belongs to a SIG when it's a commit related to PR described above (merge commit or PR's head commit).

Marking as blocked until this is confirmed.

lukaszgryglicki avatar Jun 21 '21 05:06 lukaszgryglicki

On the commits/commit comments, do you currently do that by file path? and the internal devstats mapping of which file paths belong to which sig? I was trying to find where that list is in the devstats repo but couldn't find it. >_<

Our OWNERS files tell which sig labels should be applied to PRs that touch files within those directories. e.g. https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/OWNERS would apply the labels:

  • sig/cloud-provider
  • area/provider/gcp

mrbobbytables avatar Jun 21 '21 18:06 mrbobbytables

We have per-file repo groups for kubernetes - it means that repo group can be different depending on file paths within a single repository (this is for commits, obviously).

But we don't read OWNERS - I cannot do that from SQL level, I only have file path, not its contents there, also those metrics are already quite complex, imagine joining with tables/functions that parse file contents. It would rather need to add additional functionality to parsing (git) comments, when the file name is OWNERS it should parse it and put its contents in files.

And, again, is my idea of connecting commits with PRs correct or not? Because commit and PR are separate GitHub archive events, and all I have to correlate them is SHA on the commits side and PR's head, base and merge commit (for merged PRs) SHAs on the PR side.

lukaszgryglicki avatar Jun 22 '21 04:06 lukaszgryglicki

Thanks for the path to the repo_groups file, I wasn't sure if that was it or not when I stumbled across it, it only has a couple SIG Cluster Lifecycle stuff in there. >_< I wasn't trying to push the parsing of owners files on you, I can take a follow up to update that with paths for other SIGs. I was more so just curious about how its currently done. My apologies, that wasn't clear :(

I do like your idea 👍

mrbobbytables avatar Jun 22 '21 13:06 mrbobbytables

OK so do the updates - ideally to file paths in that file, so we could avoid parsing OWNERS, please also note that I'm on PTO (the only longer PTO this year) since the weekend (two weeks).

lukaszgryglicki avatar Jun 22 '21 13:06 lukaszgryglicki

No worries, hope you have a good vacation!

mrbobbytables avatar Jun 22 '21 13:06 mrbobbytables

Thanks, maybe weren't clear enough - since the next weekend, so if we have file-level changes specified by then, I may at least start working on this.

lukaszgryglicki avatar Jun 22 '21 13:06 lukaszgryglicki