fix(stats-fetcher): user's overall commits #1691
Conversation
|
@Rongronggg9 is attempting to deploy a commit to the github readme stats Team on Vercel. A member of the Team first needs to authorize it. |
|
@anuraghazra Does this look alright to merge now? |
src/fetchers/stats-fetcher.js
Outdated
| return 0; | ||
| return { | ||
| totalPublicCommits: 0, | ||
| totalPrivateCommits: 0, |
There was a problem hiding this comment.
Maybe we can also return a success boolean and use it in
github-readme-stats/api/index.js
Line 66 in c03bb2f
To set the header?
There was a problem hiding this comment.
I choose to throw an error if any error occurs, just like what fetchStats() and totalStarsFetcher() do.
There was a problem hiding this comment.
Great! For your case, that is the best option since your using the GraphQL API!
Rongronggg9
force-pushed
the
fix/total-commits
branch
from
ee8ce91 to
ca99ad4
Compare
rickstaa
force-pushed
the
master
branch
2 times, most recently
from
86aafe8 to
8bc69e7
Compare
|
Shouldn't this be marked as bug and get a higher priority for merge? |
rickstaa
added
bug
Although this PR is correct and makes the stats' card more correct, it will break the Public Vercel instance since it costs a lot more GraphQL points (see https://docs.github.com/en/graphql/overview/resource-limitations and #1471). We therefore cannot merge this PR into the main branch. This feature will likely be included in the new GitHub action when it is released (see #2179). |
|
@Rongronggg9 @rickstaa may I ask you to fix conflicts here? I would really like to merge this PR to my fork |
|
I've rebased the PR. However, due to #2100 which introduced mistaken GraphQL objects in the test suite, it cannot pass the tests. I will open another PR to fix this before I can force-push the rebased commits. |
Well spotted. I merged your PR 👍🏻 ! |
1. precise overall public commits (formerly exaggerated) 2. precise overall private commits (formerly only the recent year) 1+2. precise overall commits including private commits
Rongronggg9
force-pushed
the
fix/total-commits
branch
from
9f26454 to
248be7c
Compare
|
I've squashed and rebased the PR @mathbunnyru |
|
Thanks @Rongronggg9 ❤️ |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1691 +/- ##
==========================================
+ Coverage 97.35% 97.41% +0.06%
==========================================
Files 24 24
Lines 4310 4341 +31
Branches 393 394 +1
==========================================
+ Hits 4196 4229 +33
+ Misses 112 110 -2
Partials 2 2
☔ View full report in Codecov by Sentry. |
1+2. precise overall commits including private commits
context: #564 (comment)
The search API is entirely unreliable and we should never use it. Though using such a traversal could potentially cause rate limits, we should still get rid of using the search API. Why? Almost all Linux kernel contributors could never use this project as you have already seen above. It is absolutely unacceptable, isn't it? If we need to avoid being rate limited, we should consider using a database to cache users' commits in the past years.
Even I and @anuraghazra are affected:
If you want to check if you are affected, my deployment is https://github-readme-stats-rongronggg9.vercel.app/api
Close #518
Close #564
Close #1061
Close #1234
Close #1515
May influence #1260, #1455