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

The script for counting commits based on organizations #1311

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eddieruan-alibaba
Copy link
Contributor

The script for counting commits based on organizations. It also help each organization to check if company name is set in associated githubids.

This is not the final script for calculating SII. We could grow that calculating tools from this script.

@eddieruan-alibaba eddieruan-alibaba requested a review from lihuay April 1, 2023 07:57
Comment on lines +202 to +204
if "cavium" in key:
return True, "CAVIUM"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if "cavium" in key:
return True, "CAVIUM"
if "cavium" in key:
return True, "CAVIUM"
if "xflow" in key:
return True, "XFLOW"

Please include xFlow Research as it's both a General Member and one of the contributing organizations. Please see https://sonic-net.github.io/SONiC/index.html for reference

@zhangyanzhao zhangyanzhao requested a review from lguohan April 5, 2023 20:29
@zhangyanzhao
Copy link
Collaborator

@eddieruan-alibaba can you please resubmit this to contributor map repo? Thanks.

"sonic-dbsyncd", "sonic-linux-kernel", "sonic-platform-common", "sonic-restapi", "sonic-swss-common", "sonic-buildimage",
"sonic-fips", "sonic-mgmt", "sonic-platform-daemons", "sonic-sairedis", "sonic-telemetry",
"sonic-frr", "sonic-mgmt-common", "sonic-py-swsssdk", "sonic-snmpagent", "sonic-utilities"
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Per TSC Election doc, the contribution count should include OCP SAI repo

Copy link
Collaborator

Choose a reason for hiding this comment

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

sonic-mgmt (test), SONiC (HLD) carry different score than other code PRs, you may need to process them separately

#
# Collect commit info from each repro
#
count_repro_commits_via_short_log(repro, years)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Besides code PR, there are many other contribution to be accounted for SII like reviews, issue triage etc, I assume they will be updated in the next patchset

#
# Company name mapping
#
def company_name_check(key):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls use contributor map posted by Guohan

"SONiC", "sonic-host-services", "sonic-mgmt-framework", "sonic-quagga", "sonic-swss", "sonic-ztp",
"sonic-dbsyncd", "sonic-linux-kernel", "sonic-platform-common", "sonic-restapi", "sonic-swss-common", "sonic-buildimage",
"sonic-fips", "sonic-mgmt", "sonic-platform-daemons", "sonic-sairedis", "sonic-telemetry",
"sonic-frr", "sonic-mgmt-common", "sonic-py-swsssdk", "sonic-snmpagent", "sonic-utilities"
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of the missing repos dhcprelay, dhcpmon and linkmgrd. Is it possible to automatically fetch the submodules from sonic-buildimage git? We can combine this with repos outside sonic-buildimage (sonic-mgmt and SONiC)

comp = comp.replace("@", "")
comp = comp.upper()
c = comp.split()
comp = c[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes that company name is always the first word. I would rather pass the entire string and check if the company's name is present in that string.

companys[comp] = count

# Total per repro
if "Total" in companys:
Copy link
Contributor

Choose a reason for hiding this comment

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

The current logic resets companys array while looping through in an upper level API. This is very hard to read. I prefer indexing companys to have repo like companys[repo][comp] and then going over this dic to print at the end

ids= [line.rstrip() for line in file]
for id in ids:
comp = get_company_from_id(id, "")
if "NOTFOUND" in comp:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be if not "NOTFOUND"?

#
# Read cached github id to company mapping if it is available
#
idfname = "/tmp/id2comp.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this as an argument

if not ret:
# Step 3: use name to find githubid,then use github id to find profile.
# Try to get company information from profile
url = "https://api.github.com/search/users?q={}".format(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Search by name is wrong. There might be many GitHub users with the same name. The search should be based on a unique key (e.g. email)

@@ -0,0 +1,495 @@
#!/usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

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

What if a specific user has quit an organization? Their github ID will no more have details of the previous organization and the final org counts will not be correct. Anyway to overcome this problem?

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.

5 participants