Skip to content

All: Add Relevanssi plugin, remove broken search algorithm #408

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 1 commit into from
Feb 14, 2018

Conversation

mgol
Copy link
Member

@mgol mgol commented Feb 11, 2018

A rebased #361

mgol pushed a commit to mgol/jquery-wp-content that referenced this pull request Feb 11, 2018
@Krinkle
Copy link
Member

Krinkle commented Feb 11, 2018

@mgol Seems fine from a quick scan and from WordPress plugin/mu perspective. As for its actual PHP code, I haven't done a complete security/integrity analysis. Neither can I say for certain that it "just work" after merely loading the files.

I'd normally expect to first install the plugin without enabling, then have the updater create the tables and create the initial index, and then enable the plugin and its ability to keep the index up-to-date.

But, the code does seem to indicate that it has the capability to do it all on-the-fly. Assuming that's all good, my only concern would be how well it is able to do that in a way that won't upset the web server during the initial requests when it is still busy rebuilding. E.g. does it use a lock to make sure only one request performs the lazy install and the rest no-ops meanwhile? That I'm not sure of.

Anyhow, lacking a better way to test it, I suppose let's just try it?

By the way, how does deployment work for jquery-wp-content? I'd hope that commits here immediately update all staging sites, but not production. And that production update either depends on a tag in this repo, or a tag in all content repos?

@mgol
Copy link
Member Author

mgol commented Feb 11, 2018

By the way, how does deployment work for jquery-wp-content? I'd hope that commits here immediately update all staging sites, but not production. And that production update either depends on a tag in this repo, or a tag in all content repos?

This is how it works for the API pages at least but I've never made sure the same applies to jquery-wp-content.

@dmethvin
Copy link
Member

I did a quick look-through, mainly to ensure there were no SQL injection issues. As long as relevanssi_variables are okay there shouldn't be a problem. I think we should just go for it.

@mgol mgol merged commit 94345a8 into jquery:master Feb 14, 2018
@mgol mgol deleted the pr-361 branch February 14, 2018 09:53
@PaulAnnekov
Copy link
Contributor

3 years... I thought it won't be ever merged.

@mgol
Copy link
Member Author

mgol commented Feb 15, 2018

@PaulAnnekov Sorry for the trouble. Our content team is seriously understaffed and the few people that maintain it are not always available.

BTW, do you perhaps know what might be causing pagination issues after landing this PR? See this comment for more details.

mgol pushed a commit that referenced this pull request Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants