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

add filename for code search #864

Merged
merged 2 commits into from
Sep 9, 2015

Conversation

fffej
Copy link
Contributor

@fffej fffej commented Aug 10, 2015

I couldn't find a helpful FileName property for code search (see https://help.github.com/articles/searching-code/#search-by-filename) , so I added it!

@shiftkey
Copy link
Member

This is really great! Thanks for picking this up!

If you feel like scoring some extra credit it'd be great to add an integration test so we can use the real API. If you're not confident, that's okay - just let me know and I'll add it to the backlog (and can add some other pointers).

@fffej
Copy link
Contributor Author

fffej commented Aug 10, 2015

I had a quick look at the tests, but didn't see an obvious way to add a good test.

The closest example to crib seems to be SearchForfunctionInCode, but this just makes a request and asserts the response is not empty. I didn't want to add another one as I'm not sure it asserts anything useful? Perhaps I'm misunderstanding the tests?

@shiftkey
Copy link
Member

I didn't want to add another one as I'm not sure it asserts anything useful?

To be honest, that's basically what I'd love to see here. Something simple to catch regressions which reflects the usage of this API.

Perhaps I'm misunderstanding the tests?

The integration tests are there to catch anything unexpected changing between the API we expect (i.e. the library) and the API we have (i.e. the server). It's something I always run as part of doing each release, so if we can keep it in sync I'll be a happy maintainer.

@fffej
Copy link
Contributor Author

fffej commented Aug 10, 2015

How's that for size?

@haacked
Copy link
Contributor

haacked commented Sep 9, 2015

crossing-thumbs-up

haacked added a commit that referenced this pull request Sep 9, 2015
@haacked haacked merged commit b858613 into octokit:master Sep 9, 2015
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.

None yet

3 participants