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

Add: Identify First-Time PR Contributors for Last Week #576

Merged

Conversation

JavidSumra
Copy link
Contributor

Fixes #153

Description

This PR implements logic to identify first-time PR contributors whose first pr_merged activity occurred within the specified time frame (last week). It ensures accuracy by filtering contributors based on their activities and checks if their first contribution happened during last week.

Screenshots / Outputs

image

Copy link

netlify bot commented Dec 25, 2024

👷 Deploy request for leaderboard-develop pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 51e1703

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

instead of directly keeping the logic here, let's add it to the contributor object itself. so that this value isNewContributor could be reused everywhere else when needed.

@rithviknishad
Copy link
Member

Also not a fan of the design. Let's not have @, seems out of context. Also placing above the name doesn't seem nice.

@JavidSumra
Copy link
Contributor Author

instead of directly keeping the logic here, let's add it to the contributor object itself. so that this value isNewContributor could be reused everywhere else when needed.

Shall I add this logic in getContributorBySlug function?

@rithviknishad
Copy link
Member

yup! that's the right place to be

lib/api.ts Outdated Show resolved Hide resolved
@JavidSumra
Copy link
Contributor Author

@rithviknishad can you please review this PR?

lib/api.ts Outdated Show resolved Hide resolved
lib/api.ts Outdated Show resolved Hide resolved
app/page.tsx Outdated Show resolved Hide resolved
app/page.tsx Outdated Show resolved Hide resolved
lib/api.ts Outdated Show resolved Hide resolved
lib/api.ts Outdated Show resolved Hide resolved
@JavidSumra
Copy link
Contributor Author

@rithviknishad can you please review this PR and clarify doubts?

app/page.tsx Outdated Show resolved Hide resolved
lib/api.ts Outdated Show resolved Hide resolved
@rithviknishad rithviknishad merged commit 46eda0a into ohcnetwork:main Jan 7, 2025
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.

Highlight new contributors that have made their first contribution in the past 1 week in landing page
2 participants