Page MenuHomePhabricator

Bundle Replace Text extension with MW 1.31
Closed, ResolvedPublic

Description

  • Passed security review or already Wikimedia deployed
  • Voting CI structure tests
  • Runs MediaWiki-CodeSniffer
  • Runs phan
  • Supports MySQL, SQLite, and Postgres (sqlite doesn't support regex, so that option is hidden)
  • GPL v2 or later compatible license
  • Extension's default configuration provides optimal experience
  • Tested with web installer

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 8 2018, 6:25 PM
Tgr added a subscriber: Tgr.Apr 8 2018, 8:56 PM
Legoktm updated the task description. (Show Details)Apr 9 2018, 2:25 AM

Oh, I just looked at this set of tasks now, and unfortunately, it turns out that Replace Text doesn't support SQLite - someone just sent me an email about that a few days ago, by coincidence. I don't know if it can- according to the guy, you can't use REGEXP and REPLACE (or SQLite's equivalents of those) together in the same query within SQLite. I don't know if that's a deal breaker if true.

Conversely, though, Replace Text does use the GPLv2.

For SQLite, I found https://stackoverflow.com/questions/4663368/custom-regexp-function-to-be-used-in-a-sqlite-select-statement that is promosing.

I have no idea if sqlite support is a requirement for being bundled.

I have no idea if sqlite support is a requirement for being bundled.

Bundled extensions should generally be compatible with most setups, and mysql/sqlite/postgres seems like a reasonable baseline. I think most problematically, the installer doesn't have any way right now to hide extensions if they don't support the database engine that was picked.

Legoktm updated the task description. (Show Details)Apr 27 2018, 1:40 AM
Yaron_Koren updated the task description. (Show Details)May 2 2018, 4:25 PM

I just checked in to Replace Text a change so that the "use regex" checkbox doesn't appear if the database is SQLite. So now Replace Text works fine with SQLite, just with more limited functionality. (Regex querying was the only thing that SQLite couldn't handle, apparently.) Is that good enough?

Sounds good to me!

I just checked in to Replace Text a change so that the "use regex" checkbox doesn't appear if the database is SQLite. So now Replace Text works fine with SQLite, just with more limited functionality. (Regex querying was the only thing that SQLite couldn't handle, apparently.) Is that good enough?

I left a comment on the patch to make it a bit more robust, but looks good to me as well. Going forward though, I think ReplaceText should go through the standard core/Wikimedia-deployed code review process (https://www.mediawiki.org/wiki/Gerrit/%2B2) and not permit self-merges.

Legoktm updated the task description. (Show Details)May 3 2018, 8:09 PM

Change 430668 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/tools/release@master] Bundle ReplaceText extension

https://gerrit.wikimedia.org/r/430668

@Legoktm - eliminating self-merges for Replace Text if it becomes part of the MediaWiki bundle is fine with me.

Change 430668 merged by jenkins-bot:
[mediawiki/tools/release@master] Bundle ReplaceText extension

https://gerrit.wikimedia.org/r/430668

Change 431534 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@REL1_31] Add ReplaceText submodule

https://gerrit.wikimedia.org/r/431534

Change 431534 merged by jenkins-bot:
[mediawiki/core@REL1_31] Add ReplaceText submodule

https://gerrit.wikimedia.org/r/431534

CCicalese_WMF closed this task as Resolved.May 7 2018, 12:40 PM
CCicalese_WMF claimed this task.
CCicalese_WMF removed a project: Patch-For-Review.