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

Remove last_active_search index #7328

Closed
wants to merge 6 commits into
base: release-2.15.1
from

Conversation

Projects
None yet
2 participants
@kuzmany
Copy link
Contributor

kuzmany commented Mar 13, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

Remove index last_active_search from this PR #6279
This index already added by FieldModel https://github.com/kuzmany/mautic/blob/928661ba87ecc8970568dce899404cd9d09fe7ae/app/bundles/LeadBundle/Model/FieldModel.php#L494-L494

Steps to test this PR:

  1. Code review is enought

alanhartless and others added some commits Mar 12, 2019

@kuzmany kuzmany added this to the 2.15.1 milestone Mar 13, 2019

@kuzmany kuzmany requested a review from alanhartless Mar 13, 2019

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Mar 13, 2019

@kuzmany is this a breaking issue in 2.15.1 beta? 2.15.1 beta is already out so unless this is a breaking issue in 2.15.1 beta, it'll have to wait till the next release.

Also, please don't assign PRs to milestones. The release leaders will manage that.

@alanhartless alanhartless removed this from the 2.15.1 milestone Mar 13, 2019

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Mar 13, 2019

Yes, it caused a lot of ugly issues with php app/console d:s:u --force remove a lot of columns and indexes. Script fail because it's duplicate index statement and everything after this line is removed from db schema.

Discussed here #7167

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Mar 14, 2019

Thank you for clarifying. I'll merge it into the beta.

@alanhartless alanhartless changed the base branch from staging to release-2.15.1 Mar 14, 2019

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Mar 14, 2019

@kuzmany can you please rebase to the release-2.15.1 branch?

Merge remote-tracking branch 'upstream/release-2.15.1' into remove-le…
…ad-entity-last-active-index

# Conflicts:
#	app/version.txt
@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Mar 14, 2019

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Mar 15, 2019

Can you rebase instead of merge? Staging has already been prepped for the next release. If we merge that into release-2.15.1, the version numbers will get all confused.

You may need to hard reset to the commit previous to your merge.

git reset --hard a534f89

Then rebase

git rebase origin/release-2.15.1

Then force push to your branch

git push --force

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Mar 15, 2019

It's ok now?

Instead of not existed
git rebase origin/release-2.15.1

I use

git rebase upstream/release-2.15.1

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Mar 20, 2019

@kuzmany doesn't seem it worked as it still includes the staging commit staging dev bump. Maybe it'll be easier to close this PR, create a new branch based on the release-2.15.1 branch, re-make the code change, then create a new PR.

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Mar 21, 2019

@kuzmany kuzmany closed this Mar 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.