Skip to content

Update version for the next release (v0.27.0) #767

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

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Conversation

meili-bot
Copy link
Contributor

@meili-bot meili-bot commented May 25, 2023

Release CHANGELOG:

⚠️ Breaking changes

🚀 Enhancements

🐛 Bug Fixes

Thanks again to @KShivendu, @alallema, @curquiza and @sanders41! 🎉

@meili-bot meili-bot added the skip-changelog The PR will not appear in the release changelogs label May 25, 2023
@brunoocasali
Copy link
Member

@alallema

  • Make params required in cancel_task why is it not a breaking-change?
  • I don't think you should mention the internal route POST /indexes/#{@uid}/documents/fetch on the get_documents also, I was not responsible for implementing the change on this repo. Please be careful in the changelogs.

Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

I left the review in the comments ;)

@sanders41
Copy link
Collaborator

sanders41 commented May 29, 2023

Make params required in cancel_task why is it not a breaking-change?

This could be viewed multiple ways.

  1. Breaking change.
  2. Bug fix. If parameters weren't sent it didn't work. This is how I discovered the issue initially. I was doing something out of the ordinary and accessed cancel_tasks directly through the TaskHandler class rather than through Client. I didn't send any params and got an error.
  3. Transparent to the end user and they will see no difference. Users typically access cancel_tasks through the Client and params was already required there.

@alallema
Copy link
Contributor

Make params required in cancel_task why is it not a breaking-change?

Because as @sanders41 said the user shouldn't see any differences. He shouldn't use it from TaskHandler and even if he did he'd get an error.
But I agreed that this PR should be on the Bug fixes section

I don't think you should mention the internal route POST /indexes/#{@uid}/documents/fetch on the get_documents also, I was not responsible for implementing the change on this repo. Please be careful in the changelogs.

@alallema alallema requested a review from brunoocasali May 29, 2023 16:20
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

LGTM ✨

@alallema alallema merged commit 863dc5b into main Jun 1, 2023
@alallema alallema deleted the meili-bot/bump-version branch June 1, 2023 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog The PR will not appear in the release changelogs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants