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

feat(javascript): provide browseObjects, browseRules and browseSynonyms helper methods #887

Merged
merged 4 commits into from
Aug 3, 2022

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Aug 1, 2022

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-601

Motivations:

(See previous motivations in the edit PR section)

We initially thought methods like browseObjects were ops related, but I discovered it was pretty used in some codebases, like 3 times in the Crawler.

The goal of this PR is to import the 3 browse* methods that were available in the version, to ease the user journey of iterating over API calls.

Changes included:

  • Move createRetryablePromise logic to a more generic createIterablePromise
    • idk about the name, I'll keep it exposed in the @algolia/client-common package as an alternative for users to do whatever they want with it
  • Add tests for new options
  • Add doc about usage breaking change

🧪 Test

  • CI :D
  • yarn docker playground javascript search or cd cd playground/javascript/node/ && yarn start:search

@shortcuts shortcuts self-assigned this Aug 1, 2022
@netlify
Copy link

netlify bot commented Aug 1, 2022

Deploy Preview for api-clients-automation ready!

Name Link
🔨 Latest commit 17558b7
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/62ea4615cf16c800099a46e1
😎 Deploy Preview https://deploy-preview-887--api-clients-automation.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@algolia-bot
Copy link
Collaborator

algolia-bot commented Aug 1, 2022

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

@shortcuts shortcuts force-pushed the feat/js-iterable-promise branch 4 times, most recently from 5e2a1da to c59ed0b Compare August 2, 2022 07:25
@shortcuts shortcuts marked this pull request as draft August 2, 2022 07:28
@shortcuts shortcuts changed the title feat(javascript): proposal - provide createIterablePromise helper feat(javascript): provide browseObjects, browseRules and browseSynonyms helper methods Aug 2, 2022
@shortcuts shortcuts marked this pull request as ready for review August 2, 2022 15:17
Copy link
Contributor

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

Looks good, minor comments 👍🏻

NB: the "how to test" part of the PR desc is there to help the reviewer, please put actual test command to run and/or repro ☺️

Copy link
Contributor

@damcou damcou left a comment

Choose a reason for hiding this comment

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

Looks good and I'm ok with the way it's done as we already discussed. I'll let someone else having a look on the typescript side of things ;)

playground/javascript/node/search.ts Outdated Show resolved Hide resolved
website/docs/clients/migration-guides/index.mdx Outdated Show resolved Hide resolved
@shortcuts shortcuts enabled auto-merge (squash) August 3, 2022 10:14
@shortcuts shortcuts merged commit 46c1526 into main Aug 3, 2022
@shortcuts shortcuts deleted the feat/js-iterable-promise branch August 3, 2022 12:53
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

4 participants