Implement methods for CSV and Ndjson batch adds #399
Conversation
|
Hello @karlomikus, PS: This message was sent automatically! |
brunoocasali
left a comment
brunoocasali
left a comment
There was a problem hiding this comment.
Hi @karlomikus, I'm so sorry for the delay in this review!
But I'm very satisfied with how you submitted this PR. It is very good for me! You can submit the next ones you mentioned in the notes section ;)
I just left two suggestions regarding the structure of the code. I think the code will be even easier to read with the suggestions.
Thanks a lot for contributing to Meilisearch! ❤️
brunoocasali
left a comment
brunoocasali
left a comment
There was a problem hiding this comment.
Thank you so much! 🤘
bors merge
399: Implement methods for CSV and Ndjson batch adds r=brunoocasali a=karlomikus # Pull Request ## Related issue Fixes #238 ## What does this PR do? - Implements methods for batch adding of documents in csv and ndjson format ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? ## Notes This is using `preg_split` and `implode` to chunk strings by newline. Let me know if there is a better way to do this. If this is fine I can implement the rest of the methods from the issue. Co-authored-by: Karlo Mikuš <[email protected]>
|
Build failed: |
|
@karlomikus I tried to merge, but we still have some issues to fix regarding the phpstan can you check that? :) |
|
bors merge |
399: Implement methods for CSV and Ndjson batch adds r=brunoocasali a=karlomikus # Pull Request ## Related issue Fixes #238 ## What does this PR do? - Implements methods for batch adding of documents in csv and ndjson format ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? ## Notes This is using `preg_split` and `implode` to chunk strings by newline. Let me know if there is a better way to do this. If this is fine I can implement the rest of the methods from the issue. Co-authored-by: Karlo Mikuš <[email protected]>
|
Build failed: |
|
I've fixed phpstan errors, I completely missed them :) I think contributing guidelines should be updated to include |
brunoocasali
left a comment
brunoocasali
left a comment
There was a problem hiding this comment.
Awesome, thanks a lot for your contribution @karlomikus 🤘
bors merge
|
Build succeeded:
|
|
This message is sent automatically Thanks again for contributing to Meilisearch ❤️ |
410: Add phpstan to contributing r=brunoocasali a=jonatanrdsantos # Pull Request Updated based on this comment #399 (comment) ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: Jonatan Santos <[email protected]>
2 participants
Pull Request
Related issue
Fixes #238
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Notes
This is using
preg_splitandimplodeto chunk strings by newline. Let me know if there is a better way to do this.If this is fine I can implement the rest of the methods from the issue.