Customization of ownerAffiliations with parameter

[developStorm]

See https://github.com/anuraghazra/github-readme-stats/issues/1#issuecomment-855681098. #1

[vercel[bot]]

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

A member of the Team first needs to authorize it.

[rickstaa]

@developStorm Thanks a lot for this pull request it works perfectly on my fork.

[tyt2y3]

Thank you so much! I find this useful 😀 I look forward to this being merged.

[rickstaa]

What is blocking this from being merged?

[rickstaa]

@developStorm I reviewed your pull request and it looks good to me. I however noticed that the tests were broken. I created a pull request to fix this on your branch https://github.com/developStorm/github-readme-stats/pull/1.

@anuraghazra Sorry for the PR waterfall. I, however, found too many changes for me to use the review suggestion tag.

[rickstaa]

Apart from these changes, I think the new argument should be added to the documentation before this pull request can be merged.

[anuraghazra]

Automated Theme Preview

• :warning: title_color does not passes AA contrast ratio
• :warning: text_color does not passes AA contrast ratio

title_color: #b57614 | icon_color: #af3a03 | text_color: #427b58 | bg_color: #fbf1c7

Preview Link

Hi, thanks for the theme contribution, please read our theme contribution guidelines.

We are currently only accepting color combinations from any VSCode theme or themes which have good color combination to minimize bloating the themes collection.

Also note that if this theme is exclusively for your personal use, then instead of adding it to our theme collection you can use card customization options

[rickstaa]

@developStorm I checked what went wrong and it looks like the changes I made in 709a65a998fd037cc45a60217079a8d80a43640b (i.e moving function input arguments) and the revert done in 4a4cec63a555cd1dd1afc79c546dff1031311481 confuse git. As a result, the diff log now also shows unrelated changes to the pull request (see https://github.com/anuraghazra/github-readme-stats/pull/1122/files). I think the best way to fix this is to use the following commands:

1. Put the pull request back to your original version before you merged my first pull request b07ce83529b7537f212cbd20183f1d2388dadcf2:

git reset --hard b07ce83529b7537f212cbd20183f1d2388dadcf2

2. Rebase this state onto the master branch:

git remote add upstream [email protected]:anuraghazra/github-readme-stats.git
git fetch upstream
git checkout master 
git rebase upstream/master

3. Cherry-pick my commits on top of this updated branch:

git cherry-pick 90d16f249e55d3698f22e5ed8d682fa121f9becb
git cherry-pick ed29f55ecf831f1d7d7d3b2cb7a6a7246ae9b4e5
git cherry-pick 1a5523ab8846d7414bf3c719addabc42dc03b2dd

4. Force push the changes to the branch connected to the developstorm/master pull request.

I can, of course, create one last pull request that you can merge after you have performed steps 1 and 2. Sorry again for the inconvenience. I have never seen git be this confused :confused:.

[rickstaa]

Great, everything seems to work now. Thanks!

[douglasg14b]

Is there something blocking?

[rickstaa]

@douglasg14b Thanks for your interest in this feature. Several things are blocking the merging of this PR.

• First, as pointed out in https://github.com/anuraghazra/github-readme-stats/issues/1#issuecomment-864451245 @anuraghazra had some concerns about adding the ORGANIZATION_MEMBER group. Although we can not prevent people from cheating, adding this group would make it too easy. I added https://github.com/anuraghazra/github-readme-stats/pull/1122#discussion_r745574651 to fix this, but both @developStorm and @anuraghazra have to accept this solution as a good middle ground.
• Second, the maxDuration should be set to 30 not 10 (see https://github.com/anuraghazra/github-readme-stats/issues/1416).
• Lastly, there are some conflicts due to changes to the master branch that need to be fixed. I can do this after the first and second points are solved.

[rickstaa]

@developStorm, @anuraghazra. @Rongronggg9 just brought another problem with including the organization stats to my attention in #1803.

Since the current GraphQL languages object returns the languages used in a repository, not those in the commits done by a specific user, the current version of the PR will make the language stats less accurate. I created a feature request to improve this behaviour on GitHub's side (see https://github.com/github-community/community/discussions/18230). You can show your support there. However, I think in #1122, for now, we should disable the ownerAffiliations option for the language card until this feature request has been implemented (see my new review).

[rickstaa]

So to summarize I think for merging this PR the following things need to be changed:

• First, as pointed out in https://github.com/anuraghazra/github-readme-stats/issues/1#issuecomment-864451245 @anuraghazra had some concerns about adding the ORGANIZATION_MEMBER group. Although we can not prevent people from cheating, adding this group would make it too easy. I added https://github.com/anuraghazra/github-readme-stats/pull/1122#discussion_r745574651 to fix this, but both @developStorm and @anuraghazra have to accept this solution as a good middle ground.
• Second, the maxDuration should be set to 30 not 10 (see https://github.com/anuraghazra/github-readme-stats/issues/1416).
• Lastly, there are some conflicts due to changes to the master branch that need to be fixed. I can do this after the first and second points are solved.
• The ownerAffiliations option needs to be temporarily disabled for the top language fetcher.

[developStorm]

@rickstaa alright thanks for letting me know. I'm gonna to sunset this PR (as it's clearly out of date from main branch) and unsubscribe #1. Please let me know if this feature gets added in the future 😀.

[rickstaa]

@developStorm, I understand. I will take the development from here. I think this PR can be merged when the changes above are applied and the documentation is updated.

@anuraghazra Since this is our most requested feature request, do you want to include #1122 with the changes above or do you think merging is still not desirable?

[developStorm]

@rickstaa The whole idea of this PR is to add support for ownerAffiliations option. However, apparently maintainers won't enable this feature and resolve #1 unless GitHub dramatically changed their API functionality to fit so called an "non-cheatable" usecase required here which is likely not gonna happen.

I can't see any point of continuing merge this or any similar PR that uses ownerAffiliations beyond than adding some useless, dangling code since they are never going to be enabled.

[rickstaa]

@developStorm Thanks for your detailed response. I have no problem merging this pull request. I have used it for months on my own GitHub readme. 🚀 That's why I have been trying to merge it into the main branch. I also don't care about cheating since, in the end, it is impossible to prevent😋. I only pointed out that:

1. I think we should filter out the ORGANIZATION_MEMBER group. Having this group enabled will also add stars of repositories in my organizations to which I never committed. I think the COLLABORATOR will group will already resolve #1.
2. It might be unwise to support ownerAffiliations for the language card due to the current implementation of the GraphQL API. The current API and the ownerAffiliations option enabled all languages in all repositories a user is a member of will be counted. This might not be a good indication of language fluency. However, if point 1 is fixed, I don't think it will be worse than the incorrect results we currently have for the language card. I think the exclude_repo option and #1732 will provide users with more than enough ways to fix their language card if they feel it is incorrect.

As a result, because this feature has been widely requested from the community since 9 Jul 2020, I'm good with merging this PR if the following things are fixed:

• [ ] https://github.com/anuraghazra/github-readme-stats/pull/1122#discussion_r745574651
• [ ] https://github.com/anuraghazra/github-readme-stats/pull/1122#discussion_r745574953
• [ ] The Vercel duration is changed back to 30 seconds.

Nevertheless, since I'm not the owner of this project, I will leave this decision to @anuraghazra.

[codecov[bot]]

Codecov Report

Patch coverage: 93.15% and project coverage change: -0.07 :warning:

Comparison is base (4b17300) 96.90% compared to head (7337505) 96.84%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1122      +/-   ##
==========================================
- Coverage   96.90%   96.84%   -0.07%     
==========================================
  Files          22       22              
  Lines        3882     3956      +74     
  Branches      337      341       +4     
==========================================
+ Hits         3762     3831      +69     
- Misses        118      123       +5     
  Partials        2        2              

Impacted Files	Coverage Δ
src/common/utils.js	98.97% <86.84%> (-1.03%)	:arrow_down:
api/index.js	96.03% <100.00%> (+0.07%)	:arrow_up:
api/top-langs.js	95.45% <100.00%> (+0.10%)	:arrow_up:
src/fetchers/stats-fetcher.js	93.40% <100.00%> (+0.26%)	:arrow_up:
src/fetchers/top-languages-fetcher.js	87.67% <100.00%> (+0.62%)	:arrow_up:

... and 1 file with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[rickstaa]

@developStorm I created #2277 as a replacement for #1122. Please take a look and close this PR if you think it suits your needs.

[rickstaa]

@anuraghazra I'm okay with merging this since it is what the community wants. As far as I know, it will not deplete our PATs since it uses the same number of GraphQL points as the master branch. It will, however, not give accurate stats results on the Public instance unless FETCH_MULTI_PAGE_STARS is enabled. It also will make the language results more incorrect since we currently get the language results from the repository object (see https://github.com/anuraghazra/github-readme-stats/pull/1122#issuecomment-1152066225). Maybe let's release this feature under the INCLUDE_ORGS environment variable? We can also remove the option entirely in the language card if you don't like the inaccuracies. With any option we still need to add:

• [ ] Tests for the new role variable.
• [ ] Merge https://github.com/anuraghazra/github-readme-stats/pull/2111 under a MULTI_PAGE_LANGUAGES env variable so that it can be used in the GitHub action.
• [ ] Maybe some documentation explaining the role variable and the language and stats card inaccuracies. We can add a section on experimental features in the readme.md.

[rickstaa]

Replaced by #2459 since I don't want to break @developStorm's master branch when I make a change.