Re: [Wikitech-l] Code Review tools
On Sun, Mar 27, 2011 at 3:33 AM, Platonides platoni...@gmail.com wrote: Daniel Friesen wrote: 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. Come on. It is easy enough to check if your revision is the culprit. svn up -r r80247 cd tests/phpunit/ make noparser Which takes approximately one hour to run. We should fix this, because otherwise nobody is going to run the unit tests before committing something. Bryan ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Meaning of fixme (Re: code review criticism (Re: Converting to Git?))
Platonides platoni...@gmail.com wrote in message news:imm5c2$rib$1...@dough.gmane.org... Ilmari Karonen wrote: I think it might be a good idea to split these two cases into separate states. My suggestion, off the top of my head, would be to leave fixme for the latter and add a new broken status for the former. +1 We should also add another state for fixmes that are not about problems in the revision itself, but request for improving more code (eg. you should fix the same thing -added in MW 1.4- in other 10 locations of the code, too). That sort of thing should be a tag, because it is orthogonal to (and can actually change independently of) the status of the revision itself. It would make it impossible to 'ok' the revision without losing the 'extend' information, which is exactly the opposite of what we want. --HM ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Code Review tools
Bryan Tong Minh wrote: On Sun, Mar 27, 2011 at 3:33 AM, Platonides wrote: Come on. It is easy enough to check if your revision is the culprit. svn up -r r80247 cd tests/phpunit/ make noparser Which takes approximately one hour to run. We should fix this, because otherwise nobody is going to run the unit tests before committing something. Bryan $ time make noparser Tests: 823, Assertions: 9512, Failures: 8, Incomplete: 42, Skipped: 3. make: *** [noparser] Error 1 real0m45.697s user0m10.389s sys 0m1.523s I have mysql tmpdir set to a tmpfs filesystem (mysql doesn't support in-memory tables with BLOBs). Using a different hardware, a cold cache and creating the temporary tables on disk, it may take a few minutes, but not an hour. On the other hand, running phpunit parser tests can take that long. Whereas the good old parserTests.php takes ~44s, too. All the other time is db overhead droping and duplicating tables, inserting articles and waiting for the db answer. I tested performing a new mysql connection instead of dropping each table separatedly, but it was slower. A change that could improve perfomance would be to insert everything on a main temporary table, and clone that with its content for each parser test. Or we could try to remove the db dependency altogether for parser tests. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Code Review tools
On Fri, Mar 25, 2011 at 8:56 PM, Mark A. Hershberger mhershber...@wikimedia.org wrote: Platonides platoni...@gmail.com writes: And no, nobody wants our review paradigm to be let's spend several months on the backlog every time we want to release. It was just the best we managed to afford. We've been doing a little better for the past month, but Robla's chart[1] is still looking ugly. So, other than switching to the mythical GIT, where all is rainbows and roses, what can we do to improve code review now? tl;dr summary: The biggest single improvement that can be made is to *ship code faster*. When new code comes in, there's basically a few things it can do: * break in an obvious and visible way * break in some circumstances, which might not be obvious on first look * mostly work, but be inefficient or have other negative side effects that need fixing * mostly work, but cause HORRIBLE DATA CORRUPTION that's not noticed for some time * work pretty well Because we're afraid of letting hard-to-find bugs go through, we're holding *everything* back for too long in the hopes that we'll somehow develop the ability to find hard-to-find bugs easily. As a result, the code paths don't get exercised until a giant last-minute review-and-push comes through months later, and finding the actual source of the bugs becomes even *more* difficult because you have 6 months of changes to search all at once instead of a few days. Ship sooner - fail faster - fix quickly. Update the live site no less frequently than weekly; update test sites more frequently than that. Make sure those test sites include things that developers are actually dogfooding. Encourage other testers to run on trunk and report issues. A smaller, but still relevant issue is to see if we can change how we think about review priorities: something that changes the core of the parser or how pages get saves might well cause HORRIBLE DATA CORRUPTION, but changes in UI code probably won't. Changes in UI code might cause an XSS vulnerability, however... so when thinking about how much attention code needs, we should be considering the module rather than laying down blanket policies. Some more explicit reviewer module 'ownership' could indeed be helpful -- especially if we have a more explicit review process for 'big changes', but even for less formal review. As far as I can see, the main reason that people think code review works better under GIT is because the committer is responsible for getting xyr[2] code reviewed *before* it is merged. The committer is motivated to find get xyr code reviewed because if xe doesn't, the code won't be used, and others will not experience its beauty. That isn't specific to git; the same methodology works in SVN or CVS or whatever where you're reviewing patches submitted through email, bug tracker systems, etc. The advantage git has here is that your intermediate work is easier to keep and share within the revision control system, as opposed to having to keep your work *outside* the version control system until it's been approved by someone else. IMO that's a big advantage, but you can still do review-first with SVN, and we always have for patches submitted through bugzilla or the mailing list by non-committers. If review and application of submitted patches can be made consistent and reasonably speedy, that would again be a big improvement without requiring a toolset change: getting more good stuff through, with no danger of it breaking things _before_ approval merging. So, while Subversion doesn't support review-first, it can incorporate revert-later. We can even use our current CodeReview tool. We just need to be more aggressive reverting unreviewed code. And just to be clear, there would be a not-too-distant “later”. I propose a week. If code is to survive past a week in the repository, it has to be reviewed. If you want to make a commit that depends on un-reviewed code, you have to find someone to review it. Otherwise, your commit will break trunk when that code is reverted. This is actually a lot harder than it might sound; even in only a week, trimming out dependency on dependency on dependency can be extremely difficult, especially if some change involved lots of giant whitespace cleanup or variable renames or other things that play hell with patch resolution. Reverting generically questionable code should probably happen a lot faster than after a week. -- brion ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Meaning of fixme (Re: code review criticism (Re: Converting to Git?))
Happy-melon wrote: Platonides platoni...@gmail.com wrote in message news:imm5c2$rib$1...@dough.gmane.org... Ilmari Karonen wrote: I think it might be a good idea to split these two cases into separate states. My suggestion, off the top of my head, would be to leave fixme for the latter and add a new broken status for the former. +1 We should also add another state for fixmes that are not about problems in the revision itself, but request for improving more code (eg. you should fix the same thing -added in MW 1.4- in other 10 locations of the code, too). That sort of thing should be a tag, because it is orthogonal to (and can actually change independently of) the status of the revision itself. It would make it impossible to 'ok' the revision without losing the 'extend' information, which is exactly the opposite of what we want. --HM Right. We should use an improve tag and stop using fixmes for that. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] The return of the Weekend Sprint to 1.17
On Fri, Mar 25, 2011 at 11:44 PM, Mark A. Hershberger mhershber...@wikimedia.org wrote: [Installer] Javascript-opened sections not open on back or error http://bugzilla.wikimedia.org/26937 [Installer] Install does not complete when choosing a CC license http://bugzilla.wikimedia.org/27170 These two are now fixed on trunk. [Installer] Chrome saves config as LocalSettings.php.php http://bugzilla.wikimedia.org/27579 Can't reproduce, closed WORKSFORME (seems to be a general Chrome issue that either does or doesn't show up depending on your system, configuration, and phase of moon?) [Installer] Cannot abort or postpone slow operations during upgrades via web interface http://bugzilla.wikimedia.org/27929 I don't think this is a new issue, isn't this the same as it's always been? While good to fix, doesn't sound like a blocker. Installer doesn't create extension tables http://bugzilla.wikimedia.org/28237 I think someone's working on this... shouldn't be too hard. Installer does not respect initial DBport declaration http://bugzilla.wikimedia.org/28162 PostgresSQL-specific -- this and other PG-specific issues should likely not block 1.17.0, though it'd be nice to have working PG at some point. Labels for DB types on page=DBConnect are too narrow http://bugzilla.wikimedia.org/28158 Can't reproduce, closed WORKSFORME. If layout and javascript are more your thing, we have a couple of those left, too: width of gallery always 100% http://bugzilla.wikimedia.org/27540 Is this really a release blocker? New wikilink window grows in width each time when opened in IE and Chrome http://bugzilla.wikimedia.org/27566 Do we ship WikiEditor on by default? If not, this probably shouldn't be a blocker. Finally, a bonus 1.17 blocker. Tim has suggested two separate ways to solve the problem. Should we go with the “simple” fix or the ”complex” fix? Truth be told, at this point, we can probably only afford the simple one. Apostrophe in linktrail breaks bolded links http://bugzilla.wikimedia.org/27473 The change to the linktrails sounds ready to go; anyone care to stick in a quick parser test case? -- brion ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
[Wikitech-l] SSL certificate expired?
Hi everyone, did the SSL certificate for secure.wikimedia.org expire? -- schnee ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] SSL certificate expired?
2011/3/27 Schneelocke schneelo...@gmail.com: Hi everyone, did the SSL certificate for secure.wikimedia.org expire? It did, yes. Informing our ops department, they should be able to sort this out quickly. Roan Kattouw (Catrope) ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Code Review tools
Brion Vibber br...@pobox.com writes: On Fri, Mar 25, 2011 at 8:56 PM, Mark A. Hershberger If you want to make a commit that depends on un-reviewed code, you have to find someone to review it. Otherwise, your commit will break trunk when that code is reverted. This is actually a lot harder than it might sound; even in only a week, trimming out dependency on dependency on dependency can be extremely difficult, especially if some change involved lots of giant whitespace cleanup or variable renames or other things that play hell with patch resolution. Reverting generically questionable code should probably happen a lot faster than after a week. I did suggest that we revert it with-in 24 hours of it being marked FIXME. I'd even be fine with immediate reversion. You suggest putting up test servers and deploying code quicker. Which I'm all in favor of. TranslateWiki does this somewhat for us, but I think setting up a prototype where this would happen more regularly and with a configuration more like WMF wikis would be a good idea. Mark. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] SSL certificate expired?
It's fixed. On Sun, Mar 27, 2011 at 2:18 PM, Roan Kattouw roan.katt...@gmail.com wrote: 2011/3/27 Schneelocke schneelo...@gmail.com: Hi everyone, did the SSL certificate for secure.wikimedia.org expire? It did, yes. Informing our ops department, they should be able to sort this out quickly. Roan Kattouw (Catrope) ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] SSL certificate expired?
Дана Sunday 27 March 2011 23:18:47 Roan Kattouw написа: 2011/3/27 Schneelocke schneelo...@gmail.com: did the SSL certificate for secure.wikimedia.org expire? It did, yes. Informing our ops department, they should be able to sort this out quickly. I hereby suggest it be renamed oops department :) ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
[Wikitech-l] HipHop
I think we should migrate MediaWiki to target HipHop [1] as its primary high-performance platform. I think we should continue to support Zend, for the benefit of small installations. But we should additionally support HipHop, use it on Wikimedia, and optimise our algorithms for it. In cases where an algorithm optimised for HipHop would be excessively slow when running under Zend, we can split the implementations by subclassing. I was skeptical about HipHop at first, since the road is littered with the bodies of dead PHP compilers. But it looks like Facebook is pretty well committed to this one, and they have the resources to maintain it. I waited and watched for a while, but I think the time has come to make a decision on this. Facebook now write their PHP code to target HipHop exclusively, so by trying to write code that works on both platforms, we'll be in new territory, to some degree. Maybe that's scary, but I think it can work. Who's with me? -- Tim Starling [1] https://github.com/facebook/hiphop-php/wiki/ ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
