On 8/27/07, Rob Church <robchur(a)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(a)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(a)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(a)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. ;) )