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

Support for version autocomplete #576

Conversation

davidackroyd99
Copy link
Contributor

Does not allow version search support for Azure yet - I have no way of testing this, when this PR is approved as being in roughly the right direction I'll try and set up a test env.

Addresses #291

@davidackroyd99 davidackroyd99 marked this pull request as draft August 23, 2020 14:48
@loic-sharma
Copy link
Owner

This looks great, thanks for doing this!

... when this PR is approved as being in roughly the right direction ...

Yup this looks like the right direction. I left a few minor nitpicks, my only real feedback is that we shouldn't reuse the AutocompleteRequest for the ListPackageVersionsAsync method as they have separate request parameters.

Please let me know if you have any questions with the Azure Search implementation! This may be a good opportunity for me to beef up our developer docs 😊

@loic-sharma
Copy link
Owner

loic-sharma commented Aug 24, 2020

Does not allow version search support for Azure yet...

By the way, I think it'd be fine for this pull request to support just the database. We can add Azure Search support later in a separate pull request.

@davidackroyd99 davidackroyd99 marked this pull request as ready for review August 24, 2020 07:33
@davidackroyd99
Copy link
Contributor Author

We can add Azure Search support later in a separate pull request.

Good idea, I've made a few more little changes and it passes the tests so if we get this in I can revisit for the Azure stuff, after we've written some integration tests for it.

@loic-sharma loic-sharma changed the base branch from master to autocomplete-versions August 29, 2020 22:22
@loic-sharma loic-sharma merged commit b3319d1 into loic-sharma:autocomplete-versions Aug 29, 2020
@loic-sharma
Copy link
Owner

Thanks for your contribution!

@loic-sharma loic-sharma mentioned this pull request Aug 29, 2020
3 tasks
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