On 8/27/07, Rob Church robchur@gmail.com wrote:
There are various reasons for this, just as there are (in addition to lack of shell users' time) reasons for the shell bugs being unfulfilled, but I think it's important to note that "need-review" means, "this bug has a patch which has not been reviewed by anyone other than the patch author" - in other words, *anyone* with a reasonable grasp of MediaWiki is free to leave comments on a possible patch.
But in practice they don't, since it's almost pointless. Getting a developer to properly review a patch is both necessary and sufficient for it to be committed. A non-developer, or developer from outside the area or who otherwise isn't comfortable committing it, does nothing by reviewing the patch, because it still has to be reviewed by a developer to be committed. They can, of course, still point out flaws and get them fixed so they're likely to be more quickly committed by a developer, but the reviewing developer can equally do that, and there's no glory in reviewing things but not committing them.
A patch may offer to solve a problem that doesn't need solving, or that shouldn't be solved in the mainline code, or it may offer a solution that hasn't really been thoroughly discussed. A patch may be unacceptable in quality (bad code practice, bad documentation, performance nightmare) or incomplete (schema change with no associated updater provided). Some of the "patches" marked on BugZilla are not, in fact, patches - comments with, "you need to change line X to line Y" aren't actually very easy to review at all, for instance.
Then if someone actually reviewed them, they could mark them WONTFIX, or review the patches as unacceptable and remove the patch keyword.
There are, no doubt, a multitude of counter-arguments to these points, many of which are fair. I think the main reminder here is that if you want your patch reviewed, *** keep pestering somebody *** - it may be that your patch was noticed, but forgotten about.
Ideally that wouldn't be necessary. If I have some time I think I'll start reviewing some of the open patches.
On 8/27/07, Yuri Astrakhan yuriastrakhan@gmail.com wrote:
What we need is a simple web-based tool that allows anyone to type in a sql query, and get the result of "EXPLAIN" statement executed on the real enwiki. Obviously we must be careful not to introduce a SQL injection vulnerability.
Never mind SQL injection:
A patch may offer to solve a problem that doesn't need solving, or that shouldn't be solved in the mainline code, or it may offer a solution that hasn't really been thoroughly discussed. A patch may be unacceptable in quality (bad code practice, bad documentation, performance nightmare) or incomplete (schema change with no associated updater provided). Some of the "patches" marked on BugZilla are not, in fact, patches - comments with, "you need to change line X to line Y" aren't actually very easy to review at all, for instance.
Then if someone actually reviewed them, they could mark them WONTFIX, or review the patches as unacceptable and remove the patch keyword.
There are, no doubt, a multitude of counter-arguments to these points, many of which are fair. I think the main reminder here is that if you want your patch reviewed, *** keep pestering somebody *** - it may be that your patch was noticed, but forgotten about.
Ideally that wouldn't be necessary. If I have some time I think I'll start reviewing some of the open patches.
On 8/27/07, Yuri Astrakhan yuriastrakhan@gmail.com wrote:
What we need is a simple web-based tool that allows anyone to type in a sql query, and get the result of "EXPLAIN" statement executed on the real enwiki. Obviously we must be careful not to introduce a SQL injection vulnerability.
Never mind SQL injection:
A patch may offer to solve a problem that doesn't need solving, or that shouldn't be solved in the mainline code, or it may offer a solution that hasn't really been thoroughly discussed. A patch may be unacceptable in quality (bad code practice, bad documentation, performance nightmare) or incomplete (schema change with no associated updater provided). Some of the "patches" marked on BugZilla are not, in fact, patches - comments with, "you need to change line X to line Y" aren't actually very easy to review at all, for instance.
Then if someone actually reviewed them, they could mark them WONTFIX, or review the patches as unacceptable and remove the patch keyword.
There are, no doubt, a multitude of counter-arguments to these points, many of which are fair. I think the main reminder here is that if you want your patch reviewed, *** keep pestering somebody *** - it may be that your patch was noticed, but forgotten about.
Ideally that wouldn't be necessary. If I have some time I think I'll start reviewing some of the open patches.
On 8/27/07, Yuri Astrakhan yuriastrakhan@gmail.com wrote:
What we need is a simple web-based tool that allows anyone to type in a sql query, and get the result of "EXPLAIN" statement executed on the real enwiki. Obviously we must be careful not to introduce a SQL injection vulnerability.
Never mind SQL injection:
mysql> EXPLAIN SELECT * FROM user WHERE user_name='Simetrical' AND user_password='d3a0021210aace9fb3550d16552ebe22'; +----+-------------+-------+------+---------------+------+---------+------+------+-----------------------------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +----+-------------+-------+------+---------------+------+---------+------+------+-----------------------------------------------------+ | 1 | SIMPLE | NULL | NULL | NULL | NULL | NULL | NULL | NULL | Impossible WHERE noticed after reading const tables | +----+-------------+-------+------+---------------+------+---------+------+------+-----------------------------------------------------+ 1 row in set (0.00 sec)
mysql> EXPLAIN SELECT * FROM user WHERE user_name='Simetrical' AND user_password='da2183e5d268388e0793759df74fbb6a'; +----+-------------+-------+-------+---------------+-----------+---------+-------+------+-------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +----+-------------+-------+-------+---------------+-----------+---------+-------+------+-------+ | 1 | SIMPLE | user | const | user_name | user_name | 257 | const | 1 | | +----+-------------+-------+-------+---------------+-----------+---------+-------+------+-------+ 1 row in set (0.00 sec)
Anyone up for designing a robust SQL sanitizer?
(That's my password on my test wiki, BTW, I think it's a single space. Don't bother whipping out the rainbow tables. ;) )