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

Use primaryLanguage instead of languages in repos per language query #33

Merged
merged 2 commits into from
Mar 21, 2021

Conversation

AleksaC
Copy link
Contributor

@AleksaC AleksaC commented Mar 20, 2021

The results of repos per language graph seemed odd to me. When I looked into it I noticed that the languageMap doesn't include many languages I have projects in. After further inspection I found out that the languages array is empty for private repos. I tried using primaryLanguages instead of languages field and it appears to be working correctly.

Here are the screenshots before and after the change.

Screenshot from 2021-03-20 17-32-14
Screenshot from 2021-03-20 17-34-37

@vn7n24fzkq
Copy link
Owner

@AleksaC Thanks for the contribution! 😄
But I cannot reproduction this situation, all of languages in my private repo are not empty in languageMap.
Probably is a github api issue, I will do more investigation to make sure we can merge this PR.

@vn7n24fzkq
Copy link
Owner

vn7n24fzkq commented Mar 21, 2021

It's odd , when use primaryLanguages instead of languages , the size of languageMap is larger. 🤔
For each repo, primaryLanguages only contains one language and languages should contains one or more.

@AleksaC
Copy link
Contributor Author

AleksaC commented Mar 21, 2021

The reason I found is that languages was an empty array for all my private repos, and since I only use some languages in private repos they don't show up in the map

@AleksaC
Copy link
Contributor Author

AleksaC commented Mar 21, 2021

Either way I think it makes more sense to use primaryLanguage, unless there are cases where it doesn't return the correct language.

@vn7n24fzkq
Copy link
Owner

vn7n24fzkq commented Mar 21, 2021

Either way I think it makes more sense to use primaryLanguage, unless there are cases where it doesn't return the correct language.

Yes, it would make more sense.
One of the reason to use languages not primaryLanguage is some languages in repo will not display if use primaryLanguage and there are some repos used more than one language, for example javascript with typescript, java with kotlin.


How many repos do you have?
If my memory serves well, the GitHub GraphQL API only allowed 100 repos per call for api pagination . And we only calculate for first 100 repos here, probably this is why the languageMap was not contains private repo languages.
We can consider to calculate for all repos.

@AleksaC
Copy link
Contributor Author

AleksaC commented Mar 21, 2021

One of the reason to use languages not primaryLanguage is some languages in repo will not display if use primaryLanguage and there are some repos used more than one language, for example javascript with typescript, java with kotlin.

This does not seem to be the case. From what I can see in the code you only include one language:

let edge = node.languages.edges[0];


We can consider to calculate for all repos.

You already do:

I own 125 repos and you include all of them.

@vn7n24fzkq
Copy link
Owner

vn7n24fzkq commented Mar 21, 2021

Oh, my mistake, seems my memory serves wrong. 😆
I think we can merge this PR right now.
But it is still weird that languages doesn't contain private repo language.
Thanks for your contributions again.

@vn7n24fzkq vn7n24fzkq merged commit 59ffc44 into vn7n24fzkq:master Mar 21, 2021
@AleksaC
Copy link
Contributor Author

AleksaC commented Mar 21, 2021

Yeah it's definitely weird, I'll see if I can a file an issue for that.

@vn7n24fzkq
Copy link
Owner

Yes, I am working for that.

@vn7n24fzkq
Copy link
Owner

vn7n24fzkq commented Mar 21, 2021

@AleksaC It is released.
v0.3.3

@AleksaC
Copy link
Contributor Author

AleksaC commented Mar 21, 2021

Thanks!

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