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

Add chainable setReverse() method to get tasks in reverse order #698

Merged
merged 9 commits into from
Dec 5, 2024

Conversation

Strift
Copy link
Collaborator

@Strift Strift commented Dec 4, 2024

Pull Request

Related issue

Fixes #693

Thank you so much for contributing to Meilisearch!

@Strift Strift linked an issue Dec 4, 2024 that may be closed by this pull request
2 tasks
@Strift Strift marked this pull request as draft December 4, 2024 10:30
@Strift
Copy link
Collaborator Author

Strift commented Dec 4, 2024

Hello @norkunas and thanks for the early review 👋 I forgot to set the PR as a draft.

I applied all your suggestions. Thanks! We're now ready for a new review.

It looks like the test is flaky though, because it failed at first; and worked when I re-ran it. I'm not sure how I can make it better. If you have feedback, it's welcome.

@Strift Strift marked this pull request as ready for review December 4, 2024 10:34
@norkunas
Copy link
Collaborator

norkunas commented Dec 4, 2024

Hello @norkunas and thanks for the early review 👋 I forgot to set the PR as a draft.

Hey 👋

It looks like the test is flaky though, because it failed at first; and worked when I re-ran it. I'm not sure how I can make it better.

Which tasks does it retrieve? Because I don't see that index would be seeded?

@norkunas
Copy link
Collaborator

norkunas commented Dec 4, 2024

Oh looks like that after each test the index is not cleared, so it sees previous tests data 😑

@Strift
Copy link
Collaborator Author

Strift commented Dec 5, 2024

Would you recommend updating the tests or do you think it's sufficient as is?

Copy link
Collaborator

@norkunas norkunas left a comment

Choose a reason for hiding this comment

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

Would you recommend updating the tests or do you think it's sufficient as is?

I'd at least add a (todo) note that these tests should be refactored to cleanup after themself

@Strift Strift merged commit 77cd241 into bump-meilisearch-v0.12.0 Dec 5, 2024
30 checks passed
@Strift Strift deleted the feat/add-tasks-reverse branch December 5, 2024 08:33
@Strift Strift changed the title Add parameter to get tasks in reverse order Add chainable setReverse() method to get tasks in reverse order Dec 11, 2024
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.

[v1.12.0] reverse parameter when querying tasks
2 participants