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

fix: total commits count #15

Closed
wants to merge 3 commits into from
Closed

fix: total commits count #15

wants to merge 3 commits into from

Conversation

anuraghazra
Copy link
Owner

So as i mentioned in #14 that github api only return commits of 1 year max. what i did is that fetched all user years and then looped through that and made a graphql request for each year

    data: {
          query: `
          query userInfo($login: String!, $from: DateTime!, $to: DateTime!) {
            user(login: $login) {
              contributionsCollection(from: $from, to: $to) {
                totalCommitContributions
              }
            }
          }
        `,
          variables: {
            login: username,
            from: currentDate.toISOString(), // current year
            to: nextDate.toISOString(), // next year
          },
        },

TODO: perf improvement?

Copy link

@anshumanv anshumanv left a comment

Choose a reason for hiding this comment

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

Looks good otherwise 💯

api/index.js Show resolved Hide resolved
@anuraghazra
Copy link
Owner Author

@anshumanv Hi, i just did some performance testing and found out that to fetch all of your profile's commits it took over ~4 seconds to get back the response..

So do you have any ideas of improving the speed?

One thing i can do is have a option for specifying if users want to fetch all of the commits like ?fetch_all_commits=true

@anshumanv
Copy link

One thing i can do is have a option for specifying if users want to fetch all of the commits like ?fetch_all_commits=true

This is definitely one but I'm not sure of cases when a user would want to limit their count, but can't think of anything else atm. This seems the best choice to me. 👍

@anshumanv
Copy link

You can give it a count param too, for the number of queries i.e no of years.

@kingthorin
Copy link
Contributor

Why not change the default text to "Current Year Commits" or "Commits for Last 365d" or something. With an option for "Total" commits along with a statement that there's a performance concern in doing such....

@TheAshwanik
Copy link

is this fixed and merged to master branch?

@anuraghazra
Copy link
Owner Author

No @TheAshwanik meanwhile you can hide the totalCommits stats if you want with hide=["commits"]

@anuraghazra
Copy link
Owner Author

anuraghazra commented Jul 13, 2020

Don't worry about rank it's not something that shows how skilled or how good you are. its just a rank for github activity. just like contribution chart. also fix would come up in this week, we need to fix the rank algo too because its biased currently

@TheAshwanik
Copy link

Don't worry about rank it's not something that shows how skilled or how good you are. its just a rank for github activity. just like contribution chart. also fix would come up in this week, we need to fix the rank algo too because its biased currently

Yes. Absolutely.
Thanks for your brilliant work.

* Add cache to function response

* Lowered cache-control maxage to 5 min

* Changed cache control directives to public, max-age

* Added cache-control to pin.js
@anuraghazra
Copy link
Owner Author

Fixed in #211

@anuraghazra anuraghazra deleted the commit-counts branch October 31, 2020 09:22
gracecarrillo pushed a commit to gracecarrillo/github-readme-stats that referenced this pull request Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stats-card Feature, Enhancement, Fixes related to stats the stats card.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants