Issue #764: Add custom year #2107

vzsky · 2022-10-04T07:34:08Z

Add custom year feature as in issue #764

now requesting /api?username=vzsky&year=2021 should work!

vercel · 2022-10-04T07:34:13Z

Someone is attempting to deploy a commit to the github readme stats Team on Vercel.

A member of the Team first needs to authorize it.

src/fetchers/stats-fetcher.js

rickstaa

Overall the idea is okay. There, however, is one bug, and the documentation and tests should be updated.

src/fetchers/stats-fetcher.js

vzsky · 2022-10-10T18:39:43Z

Edit as requested. I'm not sure if there's a better way to write the test. Please let me know if you have some advices

readme.md

Co-authored-by: Rick Staa <[email protected]>

rickstaa · 2022-10-15T07:35:03Z

@vzsky I checked your PR, and there were still some bugs. I fixed these bugs in eb445fa and af6553e. I made some comments for you to understand why I made these changes. Please review these commits 👍🏻.

One tip: You can run the project locally using the vercel dev command (see https://github.com/anuraghazra/github-readme-stats/blob/master/CONTRIBUTING.md#local-development). Doing this will allow you to debug your code locally using vscode (see vercel/vercel#2864 (comment)).

codecov · 2022-10-15T07:35:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9406466) 98.02% compared to head (de4cefa) 96.66%.
Report is 18 commits behind head on master.

❗ Current head de4cefa differs from pull request most recent head b60aa8f. Consider uploading reports for the commit b60aa8f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2107      +/-   ##
==========================================
- Coverage   98.02%   96.66%   -1.36%     
==========================================
  Files          27       22       -5     
  Lines        6275     3811    -2464     
  Branches      549      328     -221     
==========================================
- Hits         6151     3684    -2467     
- Misses        121      125       +4     
+ Partials        3        2       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tests/fetchStats.test.js

rickstaa · 2022-10-15T07:39:09Z

tests/fetchStats.test.js

@@ -253,4 +270,29 @@ describe("Test fetchStats", () => {
      rank,
    });
  });
+
+  it("should get commits of provided year", async () => {


I removed the other test, which tested whether the current year is returned when no year is supplied. I did this because this is already tested in:

github-readme-stats/tests/fetchStats.test.js

Line 127 in af6553e

it("should fetch correct stats", async () => {

github-readme-stats/tests/fetchStats.test.js

Line 216 in af6553e

it("should fetch total commits", async () => {

and

github-readme-stats/tests/fetchStats.test.js

Line 191 in af6553e

it("should fetch and add private contributions", async () => {

@anuraghazra, are you okay with this implicit testing, or do you want to have an explicit test like @vzsky did in

github-readme-stats/tests/fetchStats.test.js

Line 193 in 4b15721

it("should get present year commits when provide no year", async () => {

I did that just to be sure that the mocked function works fine. I think deleting that is probably okay since the mocked function was tested and is not likely to get changed.

tests/fetchStats.test.js

src/fetchers/stats-fetcher.js

lionkor · 2024-01-15T13:44:26Z

Maybe last year still implies "last, but not this, year". How about 1 year?

francois-rozet · 2024-01-15T14:15:38Z

(one year) or (1 year) is fine, but would need to make it uniform.

Anyway, I find myself unable to check whether the feature works or not. I am not a JS dev, and don't get how to render a "dummy" card locally 😅

rickstaa · 2024-01-15T15:59:37Z

(one year) or (1 year) is fine, but would need to make it uniform.

Anyway, I find myself unable to check whether the feature works or not. I am not a JS dev, and don't get how to render a "dummy" card locally 😅

@francois-rozet thanks for reviewing this PR ❤️‍🔥🚀. I will do the final review, including testing if it works, but your approval will give me the confidence to merge it 👍🏻.

rickstaa · 2024-01-15T16:01:49Z

(one year) or (1 year) is fine, but would need to make it uniform.

Anyway, I find myself unable to check whether the feature works or not. I am not a JS dev, and don't get how to render a "dummy" card locally 😅

I agree that it should be uniform. I used last year for the contributors count since it was clear that it meant 365 days in the past, especially since the year option will show (2024) etc. Then again, I'm not a native English speaker, so it might be better to change the text 🤔.

resolve comments

vzsky · 2024-01-15T16:47:15Z

Thanks to @francois-rozet for the comments and @rickstaa for offering to do the final review.

I have edited according to the comments from @francois-rozet.

About the display, I think both (last year) and (one year) are fine, but I prefer (one year) a little more. Also, I am also not a native. If there is a consensus then it should be easy to fix the code (and the tests?).

edit: implementation went with last year

qwerty541

Hey, @vzsky! Thanks for your pull request! I left some comments, please check it, after it will be resolved i'm okay to give my approval.

src/fetchers/stats-fetcher.js

src/cards/types.d.ts

src/fetchers/stats-fetcher.js

api/index.js

src/fetchers/stats-fetcher.js

tests/fetchStats.test.js

src/cards/stats-card.js

tests/fetchStats.test.js

Co-authored-by: Alexandr Garbuzov <[email protected]>

vercel · 2024-01-23T11:36:42Z

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

qwerty541

@vzsky Thanks for your efforts! The code looks much better now, but still needs a little polish. Please check my comments. I think that will be last changes request.

readme.md

src/cards/stats-card.js

Co-authored-by: Alexandr Garbuzov <[email protected]>

vzsky · 2024-01-25T14:25:04Z

Fixed as requested, @qwerty541.

Thanks for spotting and reporting my mistakes!

qwerty541

@vzsky No problem, thats my responsibility as collaborator to keep repository clean, thanks for your awesome contribution! I approve this pull request in it's current state. I see that @rickstaa already approved this pull request, but his review is a bit outdated. I will request a new one, but we have to wait a little since at the moment he is busy with master thesis defense, i'm counting on your patience for a little more.

1chooo · 2024-06-27T15:27:29Z

Hi @qwerty541,

I would like to ask if there are any ways to speed up the review of these PRs. This issue has been ongoing for a long time, and I hope it can be resolved because the number of commits doesn't match up. There are even cases where the total number of commits is less than the commits in the current year, which is quite odd!

If there's anything I can do to help, please let me know! Thank you to the development team for your hard work!

lionkor · 2024-06-28T12:37:32Z

2 years in review <3 truly a pinnacle of engineering

just merge it if it works, or find other maintainers.

1chooo · 2024-07-06T14:04:10Z

Hi @qwerty541,

I would like to ask if there are any ways to speed up the review of these PRs. This issue has been ongoing for a long time, and I hope it can be resolved because the number of commits doesn't match up. There are even cases where the total number of commits is less than the commits in the current year, which is quite odd!

If there's anything I can do to help, please let me know! Thank you to the development team for your hard work!

cc @rickstaa

1chooo

LGTM

Touch Sungkawichai added 2 commits October 4, 2022 16:19

add custom year

ecfd948

Merge branch 'master' of github.com:vzsky/github-readme-stats

c0ec7fc

rickstaa linked an issue Oct 8, 2022 that may be closed by this pull request

Custom Year? #764

Open

rickstaa added the hacktoberfest label Oct 8, 2022

rickstaa reviewed Oct 10, 2022

View reviewed changes

src/fetchers/stats-fetcher.js Outdated Show resolved Hide resolved

rickstaa requested changes Oct 10, 2022

View reviewed changes

src/fetchers/stats-fetcher.js Outdated Show resolved Hide resolved

src/fetchers/stats-fetcher.js Outdated Show resolved Hide resolved

Touch Sungkawichai and others added 4 commits October 11, 2022 02:54

edit fetcher

d2c4dfd

add tests

4b15721

add documentation

b74d5be

Merge branch 'master' into master

a8f8bdf

vzsky added 2 commits October 11, 2022 10:27

Update vercel.json

6fa5759

Update fetchStats.test.js

150e13a

rickstaa requested changes Oct 13, 2022

View reviewed changes

readme.md Outdated Show resolved Hide resolved

Update readme.md

15f9303

rickstaa reviewed Oct 13, 2022

View reviewed changes

readme.md Outdated Show resolved Hide resolved

vzsky and others added 3 commits October 13, 2022 21:45

Update readme.md

afb417b

Co-authored-by: Rick Staa <[email protected]>

test: fix tests

eb445fa

style: format code

af6553e

rickstaa added the hacktoberfest-accepted label Oct 15, 2022

rickstaa reviewed Oct 15, 2022

View reviewed changes

tests/fetchStats.test.js Outdated Show resolved Hide resolved

rickstaa reviewed Oct 15, 2022

View reviewed changes

tests/fetchStats.test.js Outdated Show resolved Hide resolved

rickstaa reviewed Oct 15, 2022

View reviewed changes

tests/fetchStats.test.js Outdated Show resolved Hide resolved

rickstaa reviewed Oct 15, 2022

View reviewed changes

src/fetchers/stats-fetcher.js Outdated Show resolved Hide resolved

rickstaa self-requested a review October 15, 2022 10:05

rickstaa approved these changes Oct 15, 2022

View reviewed changes

vzsky added 3 commits January 16, 2024 01:36

Update stats-fetcher.js

5b65614

resolve comments

03366e5

Merge pull request #2 from vzsky/merge_origin

0c3d572

resolve comments

qwerty541 requested changes Jan 23, 2024

View reviewed changes

vzsky and others added 4 commits January 23, 2024 19:58

Update src/cards/types.d.ts

b24733a

Co-authored-by: Alexandr Garbuzov <[email protected]>

Update src/fetchers/stats-fetcher.js

4adc788

Co-authored-by: Alexandr Garbuzov <[email protected]>

graphQL year only to commit

e729141

change argument name to commits_year

7c30597

vzsky requested review from francois-rozet and qwerty541 January 23, 2024 11:39

qwerty541 requested changes Jan 25, 2024

View reviewed changes

readme.md Outdated Show resolved Hide resolved

readme.md Outdated Show resolved Hide resolved

readme.md Outdated Show resolved Hide resolved

src/cards/stats-card.js Outdated Show resolved Hide resolved

vzsky and others added 4 commits January 25, 2024 23:11

Update readme.md

94d9612

Co-authored-by: Alexandr Garbuzov <[email protected]>

Update readme.md

e53a04e

Co-authored-by: Alexandr Garbuzov <[email protected]>

Update readme.md

4c3629b

Co-authored-by: Alexandr Garbuzov <[email protected]>

wrap label into function

b60aa8f

qwerty541 approved these changes Jan 28, 2024

View reviewed changes

qwerty541 requested a review from rickstaa January 28, 2024 17:36

qwerty541 mentioned this pull request Mar 5, 2024

fix(stats card): fetch total commits from the current year only #3683

Closed

qwerty541 mentioned this pull request Apr 10, 2024

Discrepancy in Total Commits Count for 2024 #3718

Open

1chooo approved these changes Jul 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #764: Add custom year #2107

Issue #764: Add custom year #2107

vzsky commented Oct 4, 2022

vercel bot commented Oct 4, 2022

rickstaa left a comment

vzsky commented Oct 10, 2022

rickstaa commented Oct 15, 2022 •

edited

Loading

codecov bot commented Oct 15, 2022 •

edited

Loading

rickstaa Oct 15, 2022

rickstaa Oct 15, 2022 •

edited

Loading

vzsky Oct 15, 2022

lionkor commented Jan 15, 2024

francois-rozet commented Jan 15, 2024

rickstaa commented Jan 15, 2024

rickstaa commented Jan 15, 2024

vzsky commented Jan 15, 2024 •

edited

Loading

qwerty541 left a comment

vercel bot commented Jan 23, 2024

qwerty541 left a comment

vzsky commented Jan 25, 2024

qwerty541 left a comment

1chooo commented Jun 27, 2024

lionkor commented Jun 28, 2024

1chooo commented Jul 6, 2024

1chooo left a comment

Issue #764: Add custom year #2107

Are you sure you want to change the base?

Issue #764: Add custom year #2107

Conversation

vzsky commented Oct 4, 2022

vercel bot commented Oct 4, 2022

rickstaa left a comment

Choose a reason for hiding this comment

vzsky commented Oct 10, 2022

rickstaa commented Oct 15, 2022 • edited Loading

codecov bot commented Oct 15, 2022 • edited Loading

Codecov Report

rickstaa Oct 15, 2022

Choose a reason for hiding this comment

rickstaa Oct 15, 2022 • edited Loading

Choose a reason for hiding this comment

vzsky Oct 15, 2022

Choose a reason for hiding this comment

lionkor commented Jan 15, 2024

francois-rozet commented Jan 15, 2024

rickstaa commented Jan 15, 2024

rickstaa commented Jan 15, 2024

vzsky commented Jan 15, 2024 • edited Loading

qwerty541 left a comment

Choose a reason for hiding this comment

vercel bot commented Jan 23, 2024

qwerty541 left a comment

Choose a reason for hiding this comment

vzsky commented Jan 25, 2024

qwerty541 left a comment

Choose a reason for hiding this comment

1chooo commented Jun 27, 2024

lionkor commented Jun 28, 2024

1chooo commented Jul 6, 2024

1chooo left a comment

Choose a reason for hiding this comment

rickstaa commented Oct 15, 2022 •

edited

Loading

codecov bot commented Oct 15, 2022 •

edited

Loading

rickstaa Oct 15, 2022 •

edited

Loading

vzsky commented Jan 15, 2024 •

edited

Loading