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

feat :Duplicate PR collaboration activities closes #531 #534

Merged
merged 10 commits into from
Oct 30, 2024

Conversation

VAIBHAVSING
Copy link
Contributor

here before changes this issue persist at url: http://localhost:3000/contributors/gigincg
Screenshot 2024-10-29 162509
after changes
Screenshot 2024-10-29 164253

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for leaderboard-develop ready!

Name Link
🔨 Latest commit ceeb3b8
🔍 Latest deploy log https://app.netlify.com/sites/leaderboard-develop/deploys/67221b78dfef98000885eb51
😎 Deploy Preview https://deploy-preview-534--leaderboard-develop.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

scraper/src/github-scraper/parseEvents.ts Outdated Show resolved Hide resolved
Comment on lines 12 to 24
async function fetchdefaultbranch(org: string, repo: string) {
try {
const { data } = await octokit.request('GET /repos/{owner}/{repo}', {
owner: org,
repo: repo,
});
return data.default_branch;
} catch (e) {
console.error(
`Error fetching default branch for organisation ${org} /${repo} `,
);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we maintain a cache of the results? So that we don't need to keep hitting GitHub's APIs for every PRs that collaboration event is to be added?

Comment on lines 119 to 136
if (collaborators.size > 1) {
const collaboratorArray = Array.from(collaborators); // Convert Set to Array
for (const user of collaboratorArray) {
const others = new Set(collaborators);
const othersArray = Array.from(others);

others.delete(user);
appendEvent(user, {
type: "pr_collaborated",
title: `${event.repo.name}#${event.payload.pull_request.number}`,
time: eventTime.toISOString(),
link: event.payload.pull_request.html_url,
text: event.payload.pull_request.title,
collaborated_with: [...othersArray],
});
if (event.payload.pull_request.base.ref === default_branch) {
for (const user of collaboratorArray) {
const others = new Set(collaborators);
const othersArray = Array.from(others);

others.delete(user);

appendEvent(user, {
type: "pr_collaborated",
title: `${event.repo.name}#${event.payload.pull_request.number}`,
time: eventTime.toISOString(),
link: event.payload.pull_request.html_url,
text: event.payload.pull_request.title,
collaborated_with: [...othersArray],
});
}
Copy link
Member

Choose a reason for hiding this comment

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

the check can be merged along with line before like if (collaborators.size > 1 && event.payload... === default branch right?

@@ -15,6 +15,7 @@ interface Comment {
}

export interface PullRequest {
base: any;
Copy link
Member

Choose a reason for hiding this comment

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

add proper types; any is not allowed.

@VAIBHAVSING VAIBHAVSING changed the title feat :Duplicate PR collaboration activities #531 feat :Duplicate PR collaboration activities closes #531 Oct 30, 2024
@rithviknishad rithviknishad linked an issue Oct 30, 2024 that may be closed by this pull request
Copy link
Contributor Author

@VAIBHAVSING VAIBHAVSING left a comment

Choose a reason for hiding this comment

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

fixes naming convention and managing cache for DefaultBranch to avoid extra api call and changes type from any

Comment on lines 12 to 22
const DefaultBranch = new Map<string, string>();
async function getDefaultBranch(org: string, repo: string) {
try {
if (DefaultBranch.has(`${org}/${repo}`)) {
return DefaultBranch.get(`${org}/${repo}`);
}
const { data } = await octokit.request('GET /repos/{owner}/{repo}', {
owner: org,
repo: repo,
});
return data.default_branch;
Copy link
Member

Choose a reason for hiding this comment

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

what's the use of the cache when we are not setting anything to the cache ever?

@rithviknishad rithviknishad merged commit 034d0d0 into ohcnetwork:main Oct 30, 2024
3 of 5 checks passed
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.

Duplicate PR collaboration activities
2 participants