Anonymous View
Skip to content

add country-based impact leaderboard#155

Open
ws-rush wants to merge 1 commit into
O2sa:mainfrom
ws-rush:order-top
Open

add country-based impact leaderboard#155
ws-rush wants to merge 1 commit into
O2sa:mainfrom
ws-rush:order-top

Conversation

@ws-rush

@ws-rush ws-rush commented Jun 9, 2026

Copy link
Copy Markdown

No description provided.

@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown

@ws-rush is attempting to deploy a commit to the osama's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Thank you for the pull request! ✅

A maintainer will review this soon. Please be patient while we take a look. 🙌

@ws-rush

ws-rush commented Jun 9, 2026

Copy link
Copy Markdown
Author

حاليا أي قائمة تُجلب أول مرة ثم يعتمد على الredis لجلب البيانات، قد يكون من الأفضل نجدول المهمة

@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dev-impact Ready Ready Preview, Comment Jun 10, 2026 3:06pm

@O2sa O2sa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks a lot for this PR @ws-rush

The feature direction is really valuable. A country-based leaderboard fits DevImpact very well and can make the project much more useful for discovering impactful developers by region.

I like that this PR introduces:

  • A /leaderboard page
  • Country-specific leaderboard pages
  • Country cards/grid UI
  • Leaderboard table with impact scores
  • English/Arabic localization additions
  • SEO/sitemap-related updates
  • Tests around the new behavior

However, I do not think this should be merged directly into main yet because the leaderboard feature needs more production work, especially around score calculation, caching, and GitHub API usage.

My suggestion is to merge this PR into a temporary integration branch called:

leaderboard

Then we can continue improving the leaderboard feature there before eventually merging it into main.


Main reason for using a temporary leaderboard branch

The leaderboard feature is bigger than a normal UI feature.

It needs additional work around:

  • GitHub API cost
  • Caching
  • Scheduled score calculation
  • Pagination
  • Production-safe leaderboard endpoints
  • Cache invalidation when the scoring algorithm changes
  • Handling failed/deleted GitHub users
  • Avoiding expensive live scoring during page load

So instead of blocking the whole feature until everything is perfect, we can merge this PR into the leaderboard branch after a few small/safe fixes, then continue the larger architecture work there.


Required changes before merging into the leaderboard branch

1. Disable live country scoring for now

Currently, the country leaderboard page fetches users for a country and sends them to the scoring API to calculate scores live.

This is risky because:

  • Live scoring can trigger many GitHub API calls.
  • It can hit GitHub rate limits.
  • It can hit GitHub GraphQL resource limits.
  • It can cause slow page loads.
  • It can timeout on Vercel/serverless.
  • It makes the leaderboard expensive to load repeatedly.

For now, please disable live country scoring.

The country leaderboard page can show a temporary state like:

Country leaderboard scoring will be available soon.

Alternatively, we can leave the current behavior as-is since this PR will not go to production yet, and we can improve it later inside the leaderboard branch.


2. Reduce GitHub fetch limits

The current branch appears to increase paginated fetching for PRs/issues/discussions up to a much larger amount.

For now, please reduce the fetch limits to safer values.

Suggested values:

PRs: 100
Issues: 100
Discussions: 50

Or keep the previous values if we want to be even safer:

PRs: 80
Issues: 20
Discussions: 10

I think this is a good temporary balance:

PRs: 100
Issues: 100
Discussions: 50

This keeps the scoring reasonably useful while avoiding excessive GitHub API usage.

Later, deeper fetching can be done inside scheduled jobs where we can control rate limits and caching better.


3. Localize the header leaderboard link

The header currently adds the leaderboard link as plain/hardcoded text.

Since DevImpact supports English and Arabic, please make the header link use the localization system.

For example:

{t("leaderboard.nav")}

or the correct key based on the current translation structure.

Also, please make sure the leaderboard link appears correctly in both English and Arabic.


4. Make the leaderboard link responsive on small screens

Please make sure the leaderboard navigation item is visible and usable on mobile/small screens.

Acceptance criteria:

  • Visible on desktop
  • Visible on mobile
  • Does not break the header layout

5. Remove Israel from the county list & flags


Future architecture plan after this PR is merged into leaderboard

We can discuss the remaining work and enhancements needed to make the leaderboard production-ready in this discussion: #156

@ws-rush

ws-rush commented Jun 14, 2026

Copy link
Copy Markdown
Author

Thanks a lot for this PR @ws-rush

The feature direction is really valuable. A country-based leaderboard fits DevImpact very well and can make the project much more useful for discovering impactful developers by region.

I like that this PR introduces:

  • A /leaderboard page
  • Country-specific leaderboard pages
  • Country cards/grid UI
  • Leaderboard table with impact scores
  • English/Arabic localization additions
  • SEO/sitemap-related updates
  • Tests around the new behavior

However, I do not think this should be merged directly into main yet because the leaderboard feature needs more production work, especially around score calculation, caching, and GitHub API usage.

My suggestion is to merge this PR into a temporary integration branch called:

leaderboard

Then we can continue improving the leaderboard feature there before eventually merging it into main.

Main reason for using a temporary leaderboard branch

The leaderboard feature is bigger than a normal UI feature.

It needs additional work around:

  • GitHub API cost
  • Caching
  • Scheduled score calculation
  • Pagination
  • Production-safe leaderboard endpoints
  • Cache invalidation when the scoring algorithm changes
  • Handling failed/deleted GitHub users
  • Avoiding expensive live scoring during page load

So instead of blocking the whole feature until everything is perfect, we can merge this PR into the leaderboard branch after a few small/safe fixes, then continue the larger architecture work there.

Required changes before merging into the leaderboard branch

1. Disable live country scoring for now

Currently, the country leaderboard page fetches users for a country and sends them to the scoring API to calculate scores live.

This is risky because:

  • Live scoring can trigger many GitHub API calls.
  • It can hit GitHub rate limits.
  • It can hit GitHub GraphQL resource limits.
  • It can cause slow page loads.
  • It can timeout on Vercel/serverless.
  • It makes the leaderboard expensive to load repeatedly.

For now, please disable live country scoring.

The country leaderboard page can show a temporary state like:

Country leaderboard scoring will be available soon.

Alternatively, we can leave the current behavior as-is since this PR will not go to production yet, and we can improve it later inside the leaderboard branch.

2. Reduce GitHub fetch limits

The current branch appears to increase paginated fetching for PRs/issues/discussions up to a much larger amount.

For now, please reduce the fetch limits to safer values.

Suggested values:

PRs: 100
Issues: 100
Discussions: 50

Or keep the previous values if we want to be even safer:

PRs: 80
Issues: 20
Discussions: 10

I think this is a good temporary balance:

PRs: 100
Issues: 100
Discussions: 50

This keeps the scoring reasonably useful while avoiding excessive GitHub API usage.

Later, deeper fetching can be done inside scheduled jobs where we can control rate limits and caching better.

3. Localize the header leaderboard link

The header currently adds the leaderboard link as plain/hardcoded text.

Since DevImpact supports English and Arabic, please make the header link use the localization system.

For example:

{t("leaderboard.nav")}

or the correct key based on the current translation structure.

Also, please make sure the leaderboard link appears correctly in both English and Arabic.

4. Make the leaderboard link responsive on small screens

Please make sure the leaderboard navigation item is visible and usable on mobile/small screens.

Acceptance criteria:

  • Visible on desktop
  • Visible on mobile
  • Does not break the header layout

5. Remove Israel from the county list & flags

Future architecture plan after this PR is merged into leaderboard

We can discuss the remaining work and enhancements needed to make the leaderboard production-ready in this discussion: #156

بالنسبة للfetch limit اذا خفضناها بتطلع التقييمات ناقصة، وهذي لقيتها اول شيء في حسابي، بالنسبة للlive query كما أسلفت المفروض تكون cron job اسبوعية تخزن نتيجتها في الredis ثم لا مشكلة تُجلب من الredis

@ws-rush

ws-rush commented Jun 14, 2026

Copy link
Copy Markdown
Author

Thanks a lot for this PR @ws-rush
The feature direction is really valuable. A country-based leaderboard fits DevImpact very well and can make the project much more useful for discovering impactful developers by region.
I like that this PR introduces:

  • A /leaderboard page
  • Country-specific leaderboard pages
  • Country cards/grid UI
  • Leaderboard table with impact scores
  • English/Arabic localization additions
  • SEO/sitemap-related updates
  • Tests around the new behavior

However, I do not think this should be merged directly into main yet because the leaderboard feature needs more production work, especially around score calculation, caching, and GitHub API usage.
My suggestion is to merge this PR into a temporary integration branch called:

leaderboard

Then we can continue improving the leaderboard feature there before eventually merging it into main.

Main reason for using a temporary leaderboard branch

The leaderboard feature is bigger than a normal UI feature.
It needs additional work around:

  • GitHub API cost
  • Caching
  • Scheduled score calculation
  • Pagination
  • Production-safe leaderboard endpoints
  • Cache invalidation when the scoring algorithm changes
  • Handling failed/deleted GitHub users
  • Avoiding expensive live scoring during page load

So instead of blocking the whole feature until everything is perfect, we can merge this PR into the leaderboard branch after a few small/safe fixes, then continue the larger architecture work there.

Required changes before merging into the leaderboard branch

1. Disable live country scoring for now

Currently, the country leaderboard page fetches users for a country and sends them to the scoring API to calculate scores live.
This is risky because:

  • Live scoring can trigger many GitHub API calls.
  • It can hit GitHub rate limits.
  • It can hit GitHub GraphQL resource limits.
  • It can cause slow page loads.
  • It can timeout on Vercel/serverless.
  • It makes the leaderboard expensive to load repeatedly.

For now, please disable live country scoring.
The country leaderboard page can show a temporary state like:

Country leaderboard scoring will be available soon.

Alternatively, we can leave the current behavior as-is since this PR will not go to production yet, and we can improve it later inside the leaderboard branch.

2. Reduce GitHub fetch limits

The current branch appears to increase paginated fetching for PRs/issues/discussions up to a much larger amount.
For now, please reduce the fetch limits to safer values.
Suggested values:

PRs: 100
Issues: 100
Discussions: 50

Or keep the previous values if we want to be even safer:

PRs: 80
Issues: 20
Discussions: 10

I think this is a good temporary balance:

PRs: 100
Issues: 100
Discussions: 50

This keeps the scoring reasonably useful while avoiding excessive GitHub API usage.
Later, deeper fetching can be done inside scheduled jobs where we can control rate limits and caching better.

3. Localize the header leaderboard link

The header currently adds the leaderboard link as plain/hardcoded text.
Since DevImpact supports English and Arabic, please make the header link use the localization system.
For example:

{t("leaderboard.nav")}

or the correct key based on the current translation structure.
Also, please make sure the leaderboard link appears correctly in both English and Arabic.

4. Make the leaderboard link responsive on small screens

Please make sure the leaderboard navigation item is visible and usable on mobile/small screens.
Acceptance criteria:

  • Visible on desktop
  • Visible on mobile
  • Does not break the header layout

5. Remove Israel from the county list & flags

Future architecture plan after this PR is merged into leaderboard

We can discuss the remaining work and enhancements needed to make the leaderboard production-ready in this discussion: #156

بالنسبة للfetch limit اذا خفضناها بتطلع التقييمات ناقصة، وهذي لقيتها اول شيء في حسابي، بالنسبة للlive query كما أسلفت المفروض تكون cron job اسبوعية تخزن نتيجتها في الredis ثم لا مشكلة تُجلب من الredis

وبالنسبة للاراضي المحتلة ممكن تتغير فلسطين بدل حذفها وممكن تبقى كما هي وتضاف شارة أو لافتة دعم في المشروع

@O2sa

O2sa commented Jun 14, 2026

Copy link
Copy Markdown
Owner

بالنسبة للملاحظة الخامسة الإزالة أفضل
وفيما يخص عدد العناصر التي تُجلب من الجيتهب نعم تقلليها يعني دقة أقل في التقييم وكلما زادت البيانات كان التقييم أدق، لكننا محصورين بالحدود الخاصة بالجيتهب خاصة إذا ما جعلنا عدد طلبات الدمج والقضايا والمناقشات كلها 1000 فهذا عدد كبير جدا ويجعل الخادم عرضة للأخطاء والسقوط عند أبسط ضغط لكن ممكن جعل الحد الأقصى للقضايا والمناقشات 100 عنصر بينما طلبات الدمج يمكن التسامح معها وجعلها تصل إلى 150 أو 200 لكن المشكلة أن الأمر غير مضمون وكلما زاد العدد زادت المخاطرة وربما نحتاج لوضع قيم ناتجة عن التجربة ومراقبة الأداء...

فيما يخص حساب تقييم مستخدمي الدول فالأمر كما ذكرت يتطلب جدولة لكن هذه الجدولة يجب أن يسبقها العديد من التجهيزات والقرارات التي يجب أن تتخذ وهذه النقاط التي يجب أن تؤخذ بالحسبان:

  • فيما يخص الخادم أو الخدمة التي ستقوم بحساب التقييمات يفضل أن تكون منفصلة عن الخدمة الرئيسية لكي لا تؤثر عليها، أيضا token يكون لحساب آخر وهذا يأخذنا للتفكير في هل نستنسخ ذات الخدمة أم نقوم بعمل خدمة أو script خارجي هو الذي يتولى المهمة
  • الكود الذي يقوم بحساب التقييمات يفضل أن يكون غير الكود الحالي أو هو مع إضافات تأخذ المتطلبات الجديدة بعين الإعتبار(التوقيت، المباعدة بين الطلبات، إدارة الكاش...)
  • حاليا جلب أسماء المستخدمين لكل دولة مُعتمد على موقع committers.top وربما الأفضل أن نعتمد على إستعلامنا الخاص
  • فيما يخص الكاش حاليا نحن نخزن البيانات الراجعة من الجيتهب فقط ولا نخزن نتائج التقييم، فإن توسيع نطاق المشروع ليشمل متصدري الدول يطرح علينا أسئلة عديدة منها:
  • هل نلجأ لأسهل خيار وهو تخزين البيانات بنفس الطريقة السابقة أيضأ تخزين أسماء المستخدمين لكل دولة لتجنب جلبها في كل مرة وعندما يأتي طلب نجلب بيانات المستخدمين من الكاش ونحسب تقييم كل واحد منها؟
  • كم نجعل مدة التخزين لمستخدمي الدول أم نعاملهم تماما كالمستخدمين العاديين؟
  • هل نطرح خيار إستخدام قاعدة بيانات دائمة إلى جانب الكاش؟

هذه مجموعة تساءلات لابد من مناقشتها وأغلبها مذكور في مناقشة المشروع المذكورة سابقا، نعم قد يكون بعضها تحسين أو رفاهية زائدة وليس وقتها لكن البعض الآخر مهم ولابد من آخذه بعين الإعتبار

@ws-rush أنا في إنتظار تعديلاتك السريعة المذكورة سابقا بحيث ندمج هذا الطلب للفرع المؤقت ونبدأ في إستكمال العمل المُتبقي

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.

2 participants