Roan Kattouw wrote: Let me start off with a note -- my responsibility is to keep the code clean and functional and moving forward. If something is added that I don't believe is as good as it should be, backing it out until it's been improved is a perfectly normal and virtuous bit of code maintenance, whether I'm the one who improves it or not. We give out commit access liberally not because everyone has a right to have their whims included in the trunk code, but because it streamlines the process -- it's usually a lot easier to let 90% good commits go in and revert or fix the 10% that need more work than to review 100% of patches beforehand. When that happens, it's not an attack on the committer, nor on the submitter of the patch. No malice is intended; no punishment is meted out. Hell, even I get reverted sometimes. ;) Now, to some notes specifically about that function -- The basic problem with the hook as written was that it passed in only one of the many relevant bits of a title -- the dbkey portion. There's also a namespace, an interwiki prefix, and a fragment; they would only be manipulable to the hook by poking into internal members of the title object, which isn't a clean interface and could lead to compatibility problems in the future. More generally, Title::secureAndSplit() is itself some pretty ugly code. The way it manipulates its various bits isn't very clean, and doesn't lend itself as well as it might to passing it all off to a subhook -- some variables are kept in the function, others directly manipulate private object properties. It might be desirable to do some refactoring within the function, then pass off all relevant internal variables to the hook. -- brion vibber (brion @ wikimedia.org)