-
Notifications
You must be signed in to change notification settings - Fork 5k
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
docs: Centralize Author/Team Mapping for Commit Tracking #25986
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #25986 +/- ##
=========================================
Coverage 69.67% 69.67%
=========================================
Files 1400 1401 +1
Lines 49510 49628 +118
Branches 13689 13713 +24
=========================================
+ Hits 34494 34577 +83
- Misses 15016 15051 +35 ☔ View full report in Codecov by Sentry. |
Builds ready [782fad7]
Page Load Metrics (69 ± 8 ms)
Bundle size diffs
|
Quality Gate passedIssues Measures |
Builds ready [f080167]
Page Load Metrics (144 ± 158 ms)
Bundle size diffs
|
console.log('Generation of the CSV file "commits.csv" is in progress...'); | ||
|
||
for (const commit of log.all) { | ||
const { author, message, hash } = commit; | ||
if (commitsByTeam.length >= MAX_COMMITS) { | ||
if (processedCommits >= MAX_COMMITS) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and found the current commitsByTeam.length >= MAX_COMMITS
does not work because commitsByTeam
is a object, not a Map, so it doesn't have a size property. We should track the number of processed commits by processedCommits
.
return team; | ||
} | ||
// Function to get the GitHub username for a given commit hash | ||
async function getGitHubUsername(commitHash) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the teams.json file, the PR author's username (handle) is used, so we need to retrieve the username instead of the author's name from the commit log.
Thanks @chloeYue for taking it up. I believe we can simplify the process here. When fetching the commits, we already obtain the author's information, as shown in the example below. Therefore, we don't need the fetchAuthorTeamsFile and getGitHubUsername functions. This will significantly reduce the number of Git calls. Additionally, we need the team label, which can be achieved by modifying the getPRLabels function to trim the 'team-' prefix and include it in the cvs file. Let me know if you need any help or clarification.
|
Hi @hjetpoluru, thanks for your review. We do need the |
This PR removes mapping from public repository and utilizing the team mapping centralized in a private repository, further maintaining author/team mapping in a single, unified location.
Related issues
Fixes: https://github.com/MetaMask/extension-delivery/issues/271