Skip to content

docs: update send data guide #697

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
Merged
merged 4 commits into from
Jun 15, 2022
Merged

docs: update send data guide #697

merged 4 commits into from
Jun 15, 2022

Conversation

shortcuts
Copy link
Member

🧭 What and Why

🎟 JIRA Ticket:

Changes included:

  • Update recommend guide to send data to Algolia to use the batch method instead of saveObject
  • Add migration guide for saveObjects method

Side changes:

  • Update required response fields of the batch method
  • Add more tests for the batch method (each action)

🧪 Test

Netlify preview

@shortcuts shortcuts requested a review from a team June 15, 2022 10:46
@shortcuts shortcuts self-assigned this Jun 15, 2022
@shortcuts shortcuts requested review from damcou and millotp and removed request for a team June 15, 2022 10:46
@netlify
Copy link

netlify bot commented Jun 15, 2022 •
edited
Loading

✅ Deploy Preview for api-clients-automation ready!

Name Link
🔨 Latest commit 01a6ced
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/62a9cfd42be2530009c016c8
😎 Deploy Preview https://deploy-preview-697--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.

@shortcuts shortcuts mentioned this pull request Jun 15, 2022
Merged
@algolia-bot
Copy link
Collaborator

algolia-bot commented Jun 15, 2022 •
edited
Loading

✗ 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.

millotp
millotp reviewed Jun 15, 2022
View reviewed changes
Copy link
Collaborator

@millotp millotp 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 !

website/docs/clients/migration-guides/index.mdx
<TabItem value="javascript">

```js
// The records retrieved by any of your data sources
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to repeat the documentation here ? Maybe a link is enough

Copy link
Member Author

Choose a reason for hiding this comment

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

The guide(s) should (later) be more exhaustive, so this small snippet should be enough to just show the new usage.

Ideally this page will help user gets redirected to guides etc. In this page we have the mention of the previous method names, so it pops in the search

damcou
damcou previously approved these changes Jun 15, 2022
View reviewed changes
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, the only thing a little bit weird is to have the same code sample for batch at 2 different places. Do we really need to keep the "send data to Algolia" guide ?

@shortcuts
Copy link
Member Author

Looks good, the only thing a little bit weird is to have the same code sample for batch at 2 different places. Do we really need to keep the "send data to Algolia" guide ?

(see this comment: #697 (comment))

For now the snippet and guides are pretty similar, but the guide should evolve (like an other example with a batch of 10k records (limit is 1k/batch IIRC))

The migration-guide page allows us to list the name of previous methods, so user can still search for it and get redirected to new guides/usages

@shortcuts @millotp
update java snippet
4eb5fb7 
Co-authored-by: Pierre Millot <[email protected]>
@damcou
Copy link
Contributor

damcou commented Jun 15, 2022

Looks good, the only thing a little bit weird is to have the same code sample for batch at 2 different places. Do we really need to keep the "send data to Algolia" guide ?

(see this comment: #697 (comment))

For now the snippet and guides are pretty similar, but the guide should evolve (like an other example with a batch of 10k records (limit is 1k/batch IIRC))

The migration-guide page allows us to list the name of previous methods, so user can still search for it and get redirected to new guides/usages

Ok then, let's go 🚀

millotp
millotp reviewed Jun 15, 2022
View reviewed changes
website/docs/clients/migration-guides/index.mdx Outdated
List<BatchOperation> requests = new ArrayList<>();

for (Map<String, Object> record : records) {
BatchOperation request = new BatchOperation();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you update this snippet too 🙏 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

ooops! done here 01a6ced

@shortcuts shortcuts enabled auto-merge (squash) June 15, 2022 12:26
@shortcuts shortcuts merged commit da45204 into main Jun 15, 2022
@shortcuts shortcuts deleted the docs/update-send-data-guide branch June 15, 2022 12:35
@damcou damcou mentioned this pull request Jun 20, 2022
Closed
This was referenced Jun 20, 2022
Closed
Closed
@damcou damcou mentioned this pull request Jun 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@millotp millotp millotp approved these changes

@damcou damcou damcou left review comments

Assignees

@shortcuts shortcuts

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shortcuts @algolia-bot @damcou @millotp