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.

Sign up for GitHub

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

Jump to bottom

Add filter by languages to data server requests #44

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Add filter by languages to data server requests #44

merged 1 commit into from
Dec 7, 2018

Conversation

smacker
Copy link
Contributor

@smacker smacker commented Dec 5, 2018

Ref: #36

Signed-off-by: Maxim Sukharev max@smacker.ru

@smacker smacker mentioned this pull request Dec 5, 2018
Merged
dpordomingo
dpordomingo approved these changes Dec 7, 2018
View reviewed changes
Copy link
Contributor

@dpordomingo dpordomingo left a comment

Choose a reason for hiding this comment

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

lgtm, but I'm not sure if I'd prefer include_languages for consistency.

@smacker
Copy link
Contributor Author

smacker commented Dec 7, 2018

I'm ok with both names. @carlosms wdyt?

@carlosms
Copy link
Contributor

carlosms commented Dec 7, 2018

I agree with @dpordomingo, makes sense to keep consistency and opens the possibility to have exclude_languages if we later need it.

@smacker
Copy link
Contributor Author

smacker commented Dec 7, 2018

👍 renamed

dpordomingo
dpordomingo approved these changes Dec 7, 2018
View reviewed changes
Copy link
Contributor

@dpordomingo dpordomingo left a comment

Choose a reason for hiding this comment

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

lgtm once the Carlos message was addressed
#44 (review)

@smacker
Add filter by languages to data server requests
eb8c0ee 
Ref: #36

Signed-off-by: Maxim Sukharev <max@smacker.ru>
@smacker smacker merged commit ad75847 into src-d:master Dec 7, 2018
@dpordomingo dpordomingo mentioned this pull request Jan 8, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlosms carlosms carlosms approved these changes

@dpordomingo dpordomingo dpordomingo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@smacker @carlosms @dpordomingo