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

Correctly support language filter in repo search #1951

Merged
merged 3 commits into from
Apr 1, 2019

Conversation

benmcmorran
Copy link
Contributor

@benmcmorran benmcmorran commented Mar 4, 2019

The language was being directly converted to a string through ToString() instead of using the value in the ParameterAttribute.

Copy link
Contributor

@hnrkndrssn hnrkndrssn left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ryangribble
Copy link
Contributor

Hi @benmcmorran

Did you have any more background or context on this change?

It seems github docs have changed and I cant actually find the list of language codes that are supported.
And many of our tests are using values of Csharp or Ruby which have the same name and value haha.

But even looking at your C++ example Im find that when doing searches with C++ it is actualy returning results for repo's with language of just C. Meanwhile using cpp does only find C++ language type repos. So perhaps our parameter value for the CPlusPlus entry should be cpp and not C++ anyway???

I cant seem to find the list of languages in docs anywhere anymore...

https://api.github.com/search/repositories?q=test+language:CPlusPlus (clearly wrong, there are all sorts of repos returned)
https://api.github.com/search/repositories?q=test+language:C++ (this still seems incorrect as you get C results)
https://api.github.com/search/repositories?q=test+language:cpp (this is now returning C++ results)

@hnrkndrssn
Copy link
Contributor

I think I found the list of languages https://github.com/github/linguist/blob/master/lib/linguist/languages.yml

@benmcmorran
Copy link
Contributor Author

I initially ran into this when I was attempting to write a tool to download a bunch of C/C++ repos, but most of my results ended up being Java instead. That's a good catch though about C++ not actually returning C++ results. I had only played with the web search, which does correctly handle c++ as a language. Looking back at my results, it looks like I only ended up with C repositories in my dataset.

@benmcmorran
Copy link
Contributor Author

I pushed another commit to fix the C++ case.

@ryangribble
Copy link
Contributor

we probably have other language codes that need aligning but i think in the context of this PR, the changes you've made plus fixing up the obvious c++ language code issue, should be good enough 👍

@ryangribble ryangribble merged commit 70fe726 into octokit:master Apr 1, 2019
@ryangribble
Copy link
Contributor

release_notes: Fix SearchRepositoriesRequest Language filter option to use the parameter value rather than enum member name

@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed category: bug labels Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants