On 11-03-25 11:57 PM, Ashar Voultoiz wrote:
On 26/03/11 05:48, Daniel Friesen wrote:
What about the fixmes left open since it's not clear if anything is even still broken currently.
If it is unclear: it either need a clarification or deserve a reversion. We already have enough lines hiding in the fog, read to jump at you when you get out of the path.
The fixmes for things like extra things like new tests should be added, but the actual commit in question isn't broken in any way.
The fixmes for things which are perfectly functional, but need a minor bit of tweaking since they work perfectly find, but don't use the best practice methods to do it.
Do we even have fixmes for the last two cases? Anyway for tests, they might be required just to make sure other developers using the feature will use it as intended. There are always funny corner cases to handle, specially with PHP.
I pretty much described all my commits with a fixme tagged on them:
http://www.mediawiki.org/wiki/Special:Code/MediaWiki/81928 Waiting for me to have some time to turn uses of echo into $this->output so that the built in --quiet will work, instead of my own custom implementation of --quiet (I didn't know ->output existed).
http://www.mediawiki.org/wiki/Special:Code/MediaWiki/80248 Comment gives a Tesla link saying something broke. However the Tesla link does not identify that commit as the guaranteed commit that actually broke code. The commit was followed up with several fixmes already and it's unknown if the breakage is still present. The commit is potentially perfectly functional, hit by Tesla catching a completely unrelated commit, or marking a bug that's already fixed.
http://www.mediawiki.org/wiki/Special:Code/MediaWiki/79639 Perfectly functional, just waiting for me to have time to add a small parser test for the behavior.
http://www.mediawiki.org/wiki/Special:Code/MediaWiki/79433 Of all my fixmes this one is the most bug like... that being said, it's an if() anyone could add I haven't had time to do.
http://www.mediawiki.org/wiki/Special:Code/MediaWiki/79383 The commit is perfectly functional, SkinTemplateNavigation and SkinTemplateTabs existed before and after the commit, I just replaced SkinTemplateTabs with SkinTemplateNavigation. The fixme is for the fact that Legacy skins are still using a hack that uses SkinTemplateTabs that also needs to be updated... which, to be honest isn't a good reason to revert a commit, it's pretty much orthogonal to the functionality of the commit.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]