Any extension that starts with "DynamicPageList" gives me a bad feeling :/. It looks like step 5 was skipped, I don't see the code in gerrit yet?

English Wiktionary is currently holding a vote to install this extension on that site. Note that the vote has not concluded, but looks like it will pass.

It would be great to have a preliminary review to determine whether the extension has any chance of being installed on the WMF cluster. If the overall architecture looks generally good, then we can move ahead and get the code into Gerrit. Otherwise, there is no point wasting the effort of migrating the extension to Gerrit, only to be told "No" by the arch reviewer.

For anyone willing to quickly review this, the code is currently in a zip file at Sourceforge (ugh): http://sourceforge.net/projects/mwdatatable/files/misc/DynamicPageListEngine-1.0.0.zip/download

In T90573#1727507, @TTO wrote:

English Wiktionary is currently holding a vote to install this extension on that site.
[...]
there is no point wasting the effort of migrating the extension to Gerrit, only to be told "No" by the arch reviewer.

+1. Thanks. This is important.

As a precautious measure, I have added a link to meta:Limits to configuration changes in the "See Also" section of mw:Review queue.
Note that this is not any statement on likeability/probability of extension deployment but just for informational purposes.

In T90573#1727507, @TTO wrote:

English Wiktionary is currently holding a vote to install this extension on that site. Note that the vote has not concluded, but looks like it will pass.

It would be great to have a preliminary review to determine whether the extension has any chance of being installed on the WMF cluster. If the overall architecture looks generally good, then we can move ahead and get the code into Gerrit. Otherwise, there is no point wasting the effort of migrating the extension to Gerrit, only to be told "No" by the arch reviewer.

For anyone willing to quickly review this, the code is currently in a zip file at Sourceforge (ugh): http://sourceforge.net/projects/mwdatatable/files/misc/DynamicPageListEngine-1.0.0.zip/download

Annoying, it doesn't look to be hosted in version control to begin with...

I had a quick look, it will need a decent amount of cleanup at least (why does it have ?> everywhere!?). That's without looking at architecture and performance etc

I've just initially imported it to https://github.com/reedy/DynamicPageListEngine for people to look at the code more easily. Tagged initial v1.0.0 before making any further changes

PHPStorm doesn't see anything too bad, but stuff that does want cleaning up.

Some PHP4-isms that want fixing

public function &getFeatures() {

Some manual string building for SQL queries, should be reusing MW wrappers.

	public function __construct( array $params, array &$features ) {
		parent::__construct( $params, $features,
			'imagecontainer', NS_MAIN,
			'imagelinks', 'ilx', 'il_to',
			array( 'page_title = $table.il_to',
				'page_namespace = ' . NS_FILE,
				'$table.il_from = $id' ) );
	}

Various table aliasing, many times in one query etc

I'd like to see some of the various SQL queries that the extension is likely to run, and see how they look

	/**
	 * @brief Modify a given query.
	 *
	 * Add table aliases `{$tableAlias_}1, {$tableAlias_}2, ...` to
	 * DpleQuery::$tables_.
	 *
	 * @param DpleQuery $query Query object.
	 */
	public function modifyQuery( DpleQuery &$query ) {
		$dbr = $query->getDbr();
		$tableName = $dbr->tableName( $this->tableName_ );
		$n = 1;

		/** Add conditions based on @ref $linkedTitles_. */
		for ( $i = 0; $i < $this->linkedCount_; $i++ ) {
			$table = "$tableName AS {$this->tableAlias_}$n";

			$query->addTables( $table );

			$query->addJoinCond( $table, 'INNER JOIN',
				$this->transformJoinConds_( $dbr, $n,
					$this->linkedTitles_[$i] ) );

			$n++;
		}

		/** Add conditions based on @ref $notLinkedTitles_. */
		for ( $i = 0; $i < $this->notLinkedCount_; $i++ ) {
			$table = "$tableName AS {$this->tableAlias_}$n";

			$query->addTables( $table );

			$query->addJoinCond( $table, 'LEFT OUTER JOIN',
				$this->transformJoinConds_( $dbr, $n,
					$this->notLinkedTitles_[$i] ) );

			$query->addConds(
				array( "{$this->tableAlias_}{$n}.{$this->tableColumn_}"
					=> null ) );
			$n++;
		}
	}