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.

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

Remove deprecated index creation options #2147

Merged
merged 1 commit into from
Feb 27, 2023
Merged

Remove deprecated index creation options #2147

merged 1 commit into from
Feb 27, 2023

Conversation

franmomu
Copy link
Contributor

@franmomu franmomu commented Feb 26, 2023 •
edited
Loading

Replaces #2134

Using the work done in #2134 and fixed issues found by phpstan

@franmomu franmomu force-pushed the remove-deprecated-create-options branch from f635288 to 8b63b94 Compare February 26, 2023 10:27
ruflin
ruflin reviewed Feb 27, 2023
View reviewed changes
Copy link
Owner

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Change LGTM. I'm less concerned about this change here then in #2148 because I expect create to be used less but also lets wait on the conclusion in #2148 before merging this one to potentially have it aligned.

Code LGTM.

@franmomu franmomu mentioned this pull request Feb 27, 2023
Merged
@franmomu
Copy link
Contributor Author

I think in this case is fine like this since it was deprecated to pass null, so the user has probably already changed its code.

ruflin
ruflin approved these changes Feb 27, 2023
View reviewed changes
Copy link
Owner

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Lets get this in. If there is a lot of push back we can still enable it later again.

@ruflin ruflin merged commit 323dfcf into ruflin:8.x Feb 27, 2023
@franmomu franmomu deleted the remove-deprecated-create-options branch February 27, 2023 08:09
@franmomu franmomu mentioned this pull request Mar 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@franmomu @ruflin @deguif