Skip to content

Replacement callback & row-based filtering#128

Closed
drzraf wants to merge 7458 commits into
wp-cli:masterfrom
drzraf:callback-104
Closed

Replacement callback & row-based filtering#128
drzraf wants to merge 7458 commits into
wp-cli:masterfrom
drzraf:callback-104

Conversation

@drzraf
Copy link
Copy Markdown

@drzraf drzraf commented Sep 18, 2019

ntwb and others added 30 commits April 23, 2017 12:19
Fix Travis CI caching for composer
Clarify this command as a bundled command
Use `--format=count` to only show number of rows affected
Suppress error in search-replace-export.feature.
Update package with latest scaffolded components
Use `dist: precise` for PHP 5.3, `dist: trusty` for everything else
Backtick code references so they're properly formatted on the website
Clarify "please flush object cache" message in multisite to mention cached lookup value
Add esc_sql_ident(), catering for reserved word column names.
Suppress error on fopen export file, revert PR wp-cli#16.
@schlessera schlessera added the command:search-replace Related to 'search-replace' command label Sep 27, 2019
@schlessera
Copy link
Copy Markdown
Member

@drzraf Is this still a work-in-progress ? If not, can you do the necessary style changes to make PHPCS happy? Running vendor/bin/phpcbf should already fix a large portion of them.

See here: https://travis-ci.org/wp-cli/search-replace-command/jobs/586606309#L591-L709

@drzraf
Copy link
Copy Markdown
Author

drzraf commented Apr 17, 2020

Just did^

@drzraf
Copy link
Copy Markdown
Author

drzraf commented Jul 1, 2020

Could you please have a look. It was a huge MR update and I'll hardly find the motivation to further rework/update it again if merge-conflicts arised. It's also rare to have an opportunity window to test changes.

@drzraf
Copy link
Copy Markdown
Author

drzraf commented Oct 22, 2020

ping

@schlessera
Copy link
Copy Markdown
Member

@drzraf The code looks good so far, and I'd love to merge this. However, for merging it, we'll need tests as well that cover all the main code paths. Are you able to add such tests?

@schlessera
Copy link
Copy Markdown
Member

Note: the Travis issues should be resolved by now.

@drzraf
Copy link
Copy Markdown
Author

drzraf commented Oct 29, 2020

Hi @schlessera , I completely understand your concern given the shape of the diff.
Sadly I'm not working with this command these days and have literally no-time I could dedicate to further refine the PR with test; sorry.

@schlessera
Copy link
Copy Markdown
Member

Thanks for the quick response! I'll see if I can manage to add the tests myself to get this included.

@drzraf
Copy link
Copy Markdown
Author

drzraf commented May 19, 2022

Would you merge if I were to rebase it once again?

@davidwebca
Copy link
Copy Markdown

Has this seen any update at all lately? I need to find a solution to search-replace by excluding revisions. 🤔

@danielbachhuber
Copy link
Copy Markdown
Member

@davidwebca Want to open a new pull request with this code and tests covering the change?

@davidwebca
Copy link
Copy Markdown

I'll a see what I can do!

@drzraf
Copy link
Copy Markdown
Author

drzraf commented Oct 3, 2022

@danielbachhuber : What about providing writing tests since upstream are the ones who know how to best write them?

I'd later see and possibly spend the necessary hours for another rebase.

@danielbachhuber
Copy link
Copy Markdown
Member

Proceeding with wp-cli/wp-cli#5594 for this repository. I've captured this PR to https://gist.github.com/danielbachhuber/39778759a59cb231c94520e38ae3f6ba in case this PR is auto-closed or broken in some way.

@davidwebca
Copy link
Copy Markdown

@danielbachhuber Completely had forgotten about this. Do you have any guides or examples of tests (following up that conversation higher here?)

@danielbachhuber
Copy link
Copy Markdown
Member

@davidwebca Here's our guide to writing tests: https://make.wordpress.org/cli/handbook/contributions/pull-requests/#running-and-writing-tests

There are plenty of examples within this and other repos. Feel free to stop by #cli in WordPress.org Slack with any questions you might have.

Also, specific to this pull request: I think a filter-based approach would work best for this need.

drzraf added a commit to drzraf/search-replace-command that referenced this pull request Jun 12, 2023
drzraf added a commit to drzraf/search-replace-command that referenced this pull request Oct 17, 2025
drzraf added a commit to drzraf/search-replace-command that referenced this pull request Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:search-replace Related to 'search-replace' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.