Re: [Wikitech-l] Dev Summit debrief: SOA proliferation through specification
As to JSON, IMHO YAML is better, more human-readable and less verbose I agree. As a human, YAML is rather nicer to read (and write) than JSON. Fortunately it's pretty easy to convert one to the other. We've even made some tweaks to Swagger UI to support both YAML and JSON, and we're currently generating our tests from a YAML-formatted version of our Swagger spec. On Thu, Jan 29, 2015 at 12:31 AM, Marko Obrovac mobro...@wikimedia.org wrote: On Wed, Jan 28, 2015 at 11:29 PM, Brian Gerstle bgers...@wikimedia.org wrote: JSON Schema is a recurring theme here which I'd like to encourage. I've thought it was a promising idea and would like to explore it further, both on the client and server side. If we can somehow keep data schema and API specifications separate, it would be nice to develop both of these ideas in parallel. That's exactly our idea for RESTBase/SOA. Each interface would be packaged separately as a swagger specification, which would then be required by and implemented by modules. Having such a clean and clear separation of the two would allow us to: - consult the interface independently of the implementation - have multiple modules implementing the same interface As to JSON, IMHO YAML is better, more human-readable and less verbose, but that's just a matter of personal preference - computers can read them all :P Marko On Wed, Jan 28, 2015 at 10:57 PM, Ori Livneh o...@wikimedia.org wrote: On Wed, Jan 28, 2015 at 12:30 PM, James Douglas jdoug...@wikimedia.org wrote: Howdy all, It was a pleasure chatting with you at this year's Developer Summit[1] about how we might give SOA a shot in the arm by creating (and building from) specifications. The slides are available on the RESTBase project pages[2] and the session notes are available on Etherpad[3]. Hi James, I missed your session at the developer summit, so the slides and notes are very useful. I think that having a formal specification for an API as a standalone, machine-readable document is a great idea. I have been poking at Chrome's Remote Debugging API this week and found this project, which is a cool demonstration of the power of this approach: https://github.com/cyrus-and/chrome-remote-interface The library consists of just two files: the protocol specification[0], which is represented as a JSON Schema, and the library code[1], which generates an API by walking the tree of objects and methods. This approach allows the code to be very concise. If future versions of the remote debugging protocol are published as JSON Schema files, the library could be updated without changing a single line of code. MediaWiki's API provides internal interfaces for API modules to describe their inputs and outputs, but that's not quite as powerful as having the specification truly decoupled from the code and published as a separate document. I'm glad to see that you are taking this approach with RestBASE. [0]: https://github.com/cyrus-and/chrome-remote-interface/blob/master/lib/protocol.json [1]: https://github.com/cyrus-and/chrome-remote-interface/blob/master/lib/chrome.js ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- EN Wikipedia user page: https://en.wikipedia.org/wiki/User:Brian.gerstle IRC: bgerstle ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l Marko Obrovac Senior Services Engineer Wikimedia Foundation ___ 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
[Wikitech-l] wfShellExec() quirk - advice needed on further debug
The GraphViz extension uses wfShellExec() to invoke the dot command. Sometime in the last month or so, on my Ubuntu 14.04 installation, the command started failing with: Warning: Could not load /usr/lib/graphviz/libgvplugin_gd.so.6 - file not found The file does exist and the dot command runs fine from a shell session. I found that by eliminating the ulimit -v option from ulimit4.sh, which wfShellExec() invokes, the problem goes away. The -v limit is 102400 on my installation (so 100Mb). I find it hard to believe that dot actually needs that much virtual memory. Suggestions on how to proceed with debug would be much appreciated. Thanks, Keith Welter ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] DevSummit appreciation
On 28/01/15 06:43, Erik Moeller wrote: Just a quick note that I really appreciated everyone's help making the summit come together. As always, we'll be doing lots of second-guessing of everything we did and didn't do, and how we want to use future time together. Before we go into that, I'd like to thank the event team and _everyone_ who worked to and beyond the point of exhaustion to organize the event, support attendees, plan sessions, facilitate conversations, negotiate sometimes difficult terrain. Thank you. :) Ys, this. It was awesome and there were issues, but there are always issues and when none of the issues were problematic that's great. If that makes sense; I still haven't slept after getting back from it. But you know what I mean. Seriously, thank you to everyone - those who put in all the work to make it happen, and those who were there and did stuff and things and things and stuff and I'm babbling yay why am I even awake. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Dev Summit debrief: SOA proliferation through specification
On Wed, Jan 28, 2015 at 11:29 PM, Brian Gerstle bgers...@wikimedia.org wrote: JSON Schema is a recurring theme here which I'd like to encourage. I've thought it was a promising idea and would like to explore it further, both on the client and server side. If we can somehow keep data schema and API specifications separate, it would be nice to develop both of these ideas in parallel. That's exactly our idea for RESTBase/SOA. Each interface would be packaged separately as a swagger specification, which would then be required by and implemented by modules. Having such a clean and clear separation of the two would allow us to: - consult the interface independently of the implementation - have multiple modules implementing the same interface As to JSON, IMHO YAML is better, more human-readable and less verbose, but that's just a matter of personal preference - computers can read them all :P Marko On Wed, Jan 28, 2015 at 10:57 PM, Ori Livneh o...@wikimedia.org wrote: On Wed, Jan 28, 2015 at 12:30 PM, James Douglas jdoug...@wikimedia.org wrote: Howdy all, It was a pleasure chatting with you at this year's Developer Summit[1] about how we might give SOA a shot in the arm by creating (and building from) specifications. The slides are available on the RESTBase project pages[2] and the session notes are available on Etherpad[3]. Hi James, I missed your session at the developer summit, so the slides and notes are very useful. I think that having a formal specification for an API as a standalone, machine-readable document is a great idea. I have been poking at Chrome's Remote Debugging API this week and found this project, which is a cool demonstration of the power of this approach: https://github.com/cyrus-and/chrome-remote-interface The library consists of just two files: the protocol specification[0], which is represented as a JSON Schema, and the library code[1], which generates an API by walking the tree of objects and methods. This approach allows the code to be very concise. If future versions of the remote debugging protocol are published as JSON Schema files, the library could be updated without changing a single line of code. MediaWiki's API provides internal interfaces for API modules to describe their inputs and outputs, but that's not quite as powerful as having the specification truly decoupled from the code and published as a separate document. I'm glad to see that you are taking this approach with RestBASE. [0]: https://github.com/cyrus-and/chrome-remote-interface/blob/master/lib/protocol.json [1]: https://github.com/cyrus-and/chrome-remote-interface/blob/master/lib/chrome.js ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- EN Wikipedia user page: https://en.wikipedia.org/wiki/User:Brian.gerstle IRC: bgerstle ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l Marko Obrovac Senior Services Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Thoughts: stateless services with open servers?
Hi! I believe we can make installing a fully-featured MediaWiki service system as simple as copypasting 2-3 lines to a shell, or even executing a remote I think this is underestimating the issue, unfortunately. I.e., in ideal situation - the user runs the same system, with the same services and permissions, that the shell lines were tested on, it probably would work pretty good. But what if he doesn't? What if he doesn't have root access? Has non-Linux Unix system? Has custom-built VPS system with custom-tailored access/capability permissions? Has Windows (not talking about more exotic systems that do have PHP support)? Has no direct Internet access? Has security settings that further restrict their capabilities? Has some of the libraries/packages that our dependencies' dependencies installed in different versions that support different options/APIs/command-line parameters? The problem would be not in those 3 shell lines, but in the complexity those lines hide and in the assumptions those 3 lines make. The difference is between if this system runs PHP, you're OK to if this system can run this three scripts which runs a dozen of scripts which invoke a dozen of software packages with an unknown number of dependencies each and all of them are OK then you're OK. The point is more moving parts you add, more potential failures are there, and these potential failures very soon become real ones when users start using it in a wide wild world. And my concern here is that the users - and, more importantly, potential contributors - faced with these failures, would do what James mentioned - pack it and move on to something else, which still is if you have running PHP, you're OK. installer script that runs those lines for you. Additionally, we can offer VM images derived from the same install process through Bitnami or others. VM solves the dependency/environment issue, but running VM on a constant basis requires some sysadmin skills and knowledge (and much higher hardware/access requirements) which again means people with no sysadmin skills would be cut off. -- Stas Malyshev smalys...@wikimedia.org ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] wfShellExec() quirk - advice needed on further debug
Brion, Doubling the memory limit resolved my problem. Thanks for the suggestion (and the quick response). Keith Welter On Thu, Jan 29, 2015 at 9:58 AM, Brion Vibber bvib...@wikimedia.org wrote: Memory limits are notoriously difficult to deal with on Unix like systems. Virtual memory address space can easily be exhausted mapping in the program itself, its libraries, and sometimes even scratch files might be memory mapped, eating address space up further. Usage also tends to be higher on 64-bit machines than 32, which was I think still common when we set the defaults. Probably we should just double the default and see if that's more reliable... -- brion On Jan 29, 2015 9:27 AM, Keith Welter welte...@gmail.com wrote: The GraphViz extension uses wfShellExec() to invoke the dot command. Sometime in the last month or so, on my Ubuntu 14.04 installation, the command started failing with: Warning: Could not load /usr/lib/graphviz/libgvplugin_gd.so.6 - file not found The file does exist and the dot command runs fine from a shell session. I found that by eliminating the ulimit -v option from ulimit4.sh, which wfShellExec() invokes, the problem goes away. The -v limit is 102400 on my installation (so 100Mb). I find it hard to believe that dot actually needs that much virtual memory. Suggestions on how to proceed with debug would be much appreciated. Thanks, Keith Welter ___ 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 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] wfShellExec() quirk - advice needed on further debug
Memory limits are notoriously difficult to deal with on Unix like systems. Virtual memory address space can easily be exhausted mapping in the program itself, its libraries, and sometimes even scratch files might be memory mapped, eating address space up further. Usage also tends to be higher on 64-bit machines than 32, which was I think still common when we set the defaults. Probably we should just double the default and see if that's more reliable... -- brion On Jan 29, 2015 9:27 AM, Keith Welter welte...@gmail.com wrote: The GraphViz extension uses wfShellExec() to invoke the dot command. Sometime in the last month or so, on my Ubuntu 14.04 installation, the command started failing with: Warning: Could not load /usr/lib/graphviz/libgvplugin_gd.so.6 - file not found The file does exist and the dot command runs fine from a shell session. I found that by eliminating the ulimit -v option from ulimit4.sh, which wfShellExec() invokes, the problem goes away. The -v limit is 102400 on my installation (so 100Mb). I find it hard to believe that dot actually needs that much virtual memory. Suggestions on how to proceed with debug would be much appreciated. Thanks, Keith Welter ___ 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] Performance of parsing links?
As of https://gerrit.wikimedia.org/r/#/c/29879/2/utils/MessageTable.php,cm , Linker::link took 20 KiB of memory per call. Cf. http://laxstrom.name/blag/2013/02/01/how-i-debug-performance-issues-in-mediawiki/ I don't know if such bugs/unfeatures and related best practices were written down somewhere. Nemo ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
How about a simple script to create a phabricator task after a few days (a week?) of a patch inactivity to review that patch. It will allow assign to, allow managers to see each dev's review queue, and will prevent patches to fall through the cracks. Obviously this won't be needed after we move gerrit to phabricator. On Thu, Jan 29, 2015 at 1:44 PM, James Douglas jdoug...@wikimedia.org wrote: This is a situation where disciplined testing can come in really handy. If I submit a patch, and the patch passes the tests that have been specified for the feature it implements (or the bug it fixes), and the code coverage is sufficiently high, then a reviewer has a running start in terms of confidence in the correctness and completeness of the patch. Practically speaking, this doesn't necessarily rely on rest of the project already having a very level of code coverage; as long as there are tests laid out for the feature in question, and the patch makes those tests pass, it gives the code reviewer a real shot in the arm. On Thu, Jan 29, 2015 at 1:14 PM, Jon Robson jdlrob...@gmail.com wrote: Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) bjor...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk * https://www.facebook.com/jonrobson * @rakugojon ___ 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 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) bjor...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk * https://www.facebook.com/jonrobson * @rakugojon ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
This is a situation where disciplined testing can come in really handy. If I submit a patch, and the patch passes the tests that have been specified for the feature it implements (or the bug it fixes), and the code coverage is sufficiently high, then a reviewer has a running start in terms of confidence in the correctness and completeness of the patch. Practically speaking, this doesn't necessarily rely on rest of the project already having a very level of code coverage; as long as there are tests laid out for the feature in question, and the patch makes those tests pass, it gives the code reviewer a real shot in the arm. On Thu, Jan 29, 2015 at 1:14 PM, Jon Robson jdlrob...@gmail.com wrote: Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) bjor...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk * https://www.facebook.com/jonrobson * @rakugojon ___ 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] Improving our code review efficiency
I'd like us to start by using the review request system already in gerrit more fully. Personally I've got a bunch of incoming reviews in my queue where I'm not sure the current status of them or if it's wise to land them. :) First step is probably to go through the existing old patches in everybody's queues and either do the review, abandon the patch, or trim down reviewers who aren't familiar with the code area. Rejected patches should be abandoned to get them out of the queues. Then we should go through unassigned patches more aggressively... We also need to make sure we have default reviewers for modules, which will be relevant also to triaging bug reports. -- brion On Jan 29, 2015 2:03 PM, Yuri Astrakhan yastrak...@wikimedia.org wrote: How about a simple script to create a phabricator task after a few days (a week?) of a patch inactivity to review that patch. It will allow assign to, allow managers to see each dev's review queue, and will prevent patches to fall through the cracks. Obviously this won't be needed after we move gerrit to phabricator. On Thu, Jan 29, 2015 at 1:44 PM, James Douglas jdoug...@wikimedia.org wrote: This is a situation where disciplined testing can come in really handy. If I submit a patch, and the patch passes the tests that have been specified for the feature it implements (or the bug it fixes), and the code coverage is sufficiently high, then a reviewer has a running start in terms of confidence in the correctness and completeness of the patch. Practically speaking, this doesn't necessarily rely on rest of the project already having a very level of code coverage; as long as there are tests laid out for the feature in question, and the patch makes those tests pass, it gives the code reviewer a real shot in the arm. On Thu, Jan 29, 2015 at 1:14 PM, Jon Robson jdlrob...@gmail.com wrote: Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) bjor...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk * https://www.facebook.com/jonrobson * @rakugojon ___ 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 ___ 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
[Wikitech-l] Improving our code review efficiency
I was really happy to hear Damon, at the MediaWiki Developer Summit, ask us how long we take to code review and whether we had communicated a timeframe in which we promised to do it to our community. He quite rightly stressed that this was vital for the survival of our community. I spoke to one of our more new developers during the summit and he also confessed to me that the reason he was an active volunteer in our extension was that he got feedback on his code pretty quickly. I had a few ideas about how to measure this so in my spare time I have generated this report based on data from Gerrit patchsets using a hacked together python script [1] which I hope will be if nothing else an interesting artifact to talk about and generate some discussion. Introducing: https://www.mediawiki.org/wiki/Extension_health To help you understand what you are reading, let's take Echo as an example: Project: mediawiki/extensions/Echo 524 patches analysed (23 open, 501 merged) Average review time: 29 days Oldest open patch: (bug 41987) Updating tables indexes' names. (766 days) - https://gerrit.wikimedia.org/r/40095 The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. There are many patches like this outside Echo. We should probably be seeing those patchsets through to completion or abandoning them on the basis that if it hasn't been merged in over 2 years it's probably not important and shouldn't be clogging up people's review queues and hiding other more important patchsets which need review. The more troubling situation is when patches have been open for some time and have not been reviewed at all or are awaiting for some response... let's get better at this! Help make this ecosystem better. I will rerun the script in a month and see if we have improved. Note our average time to review across all those projects seems to be 18 days. That's really not good. We can do better. We will do better. [1] https://github.com/jdlrobson/GerritCommandLine ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
Brion, i would love to use gerrit more fully (that is until we finally migrate! :)), but gerrit to my knowledge does not differentiate between a CC (review if you want to) and TO (i want you to +2). Having multiple cooks means some patches don't get merged at all. I feel each patch should be assigned to a person who will +2 it. This does not preclude others from +2ing it, but it designates one responsible for the answer. On Thu, Jan 29, 2015 at 2:33 PM, Brion Vibber bvib...@wikimedia.org wrote: I'd like us to start by using the review request system already in gerrit more fully. Personally I've got a bunch of incoming reviews in my queue where I'm not sure the current status of them or if it's wise to land them. :) First step is probably to go through the existing old patches in everybody's queues and either do the review, abandon the patch, or trim down reviewers who aren't familiar with the code area. Rejected patches should be abandoned to get them out of the queues. Then we should go through unassigned patches more aggressively... We also need to make sure we have default reviewers for modules, which will be relevant also to triaging bug reports. -- brion On Jan 29, 2015 2:03 PM, Yuri Astrakhan yastrak...@wikimedia.org wrote: How about a simple script to create a phabricator task after a few days (a week?) of a patch inactivity to review that patch. It will allow assign to, allow managers to see each dev's review queue, and will prevent patches to fall through the cracks. Obviously this won't be needed after we move gerrit to phabricator. On Thu, Jan 29, 2015 at 1:44 PM, James Douglas jdoug...@wikimedia.org wrote: This is a situation where disciplined testing can come in really handy. If I submit a patch, and the patch passes the tests that have been specified for the feature it implements (or the bug it fixes), and the code coverage is sufficiently high, then a reviewer has a running start in terms of confidence in the correctness and completeness of the patch. Practically speaking, this doesn't necessarily rely on rest of the project already having a very level of code coverage; as long as there are tests laid out for the feature in question, and the patch makes those tests pass, it gives the code reviewer a real shot in the arm. On Thu, Jan 29, 2015 at 1:14 PM, Jon Robson jdlrob...@gmail.com wrote: Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) bjor...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk * https://www.facebook.com/jonrobson * @rakugojon ___ 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 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org
[Wikitech-l] Urlencoding strip markers
Currently, while {{urlencod}}ing, content in strip markers is skipped.
I believe this violates the expectation that the entire output
will be properly escaped to be placed in a sensitive context.
An example is in the infobox book caption on,
https://en.wikipedia.org/wiki/%22F%22_Is_for_Fugitive
There’s a brief discussions of the security implications of
some proposed solutions in the review of,
https://gerrit.wikimedia.org/r/#/c/181519/
It seems best (I guess) to just drop the content (`killMarkers()`).
Any opinions or better ideas?
Thanks,
Arlo
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
As I mentioned to Nemo on the talk page, I want an easy way to see how my code review efficiency compares to other projects and to see which projects are getting more love than others. A few thoughts: 1) From http://korma.wmflabs.org/browser/repository.html?repository=gerrit.wikimedia.org_mediawiki_extensions_MobileFrontend I can see it takes 3.2 days to get a review (I think - there are too many numbers to look at and no key to tell the difference) I can see on Echo http://korma.wmflabs.org/browser/repository.html?repository=gerrit.wikimedia.org_mediawiki_extensions_Echo it is 10.7 days but want I really want is to see a league table type thing to tell where we are giving more attention compared to other projects. 2) Also I think an average review time is only really useful if it is based on data from the last month. 3) What about open patchsets - does average review time take into account that some patches still haven't got merged? If a patch has been sitting around for 100 days, I care more about this then an existing patch that got merged after 5 days. These should impact the average. 4) Also this dashboard is not actionable and has no call to action - why don't we show the most neglected patchsets on each page and encourage people to actually go and review them! On Thu, Jan 29, 2015 at 3:38 PM, Quim Gil q...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: Introducing: https://www.mediawiki.org/wiki/Extension_health Interesting! I'm hopping between flights back to Europe, and I don't have time to review these metrics more carefully, but please check http://korma.wmflabs.org/browser/gerrit_review_queue.html and let me know what you miss. -- Quim Gil Engineering Community Manager @ Wikimedia Foundation http://www.mediawiki.org/wiki/User:Qgil ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk * https://www.facebook.com/jonrobson * @rakugojon ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
Good point Yuri -- a lot of those items on my queue are assigned to several reviewers so none of us feels ownership, and that's definitely part of the reason some of them sit around so long. A regular bot run that assigns untouched review requests to a single person in Phab probably does make sense... -- brion On Thu, Jan 29, 2015 at 2:48 PM, Yuri Astrakhan yastrak...@wikimedia.org wrote: Brion, i would love to use gerrit more fully (that is until we finally migrate! :)), but gerrit to my knowledge does not differentiate between a CC (review if you want to) and TO (i want you to +2). Having multiple cooks means some patches don't get merged at all. I feel each patch should be assigned to a person who will +2 it. This does not preclude others from +2ing it, but it designates one responsible for the answer. On Thu, Jan 29, 2015 at 2:33 PM, Brion Vibber bvib...@wikimedia.org wrote: I'd like us to start by using the review request system already in gerrit more fully. Personally I've got a bunch of incoming reviews in my queue where I'm not sure the current status of them or if it's wise to land them. :) First step is probably to go through the existing old patches in everybody's queues and either do the review, abandon the patch, or trim down reviewers who aren't familiar with the code area. Rejected patches should be abandoned to get them out of the queues. Then we should go through unassigned patches more aggressively... We also need to make sure we have default reviewers for modules, which will be relevant also to triaging bug reports. -- brion On Jan 29, 2015 2:03 PM, Yuri Astrakhan yastrak...@wikimedia.org wrote: How about a simple script to create a phabricator task after a few days (a week?) of a patch inactivity to review that patch. It will allow assign to, allow managers to see each dev's review queue, and will prevent patches to fall through the cracks. Obviously this won't be needed after we move gerrit to phabricator. On Thu, Jan 29, 2015 at 1:44 PM, James Douglas jdoug...@wikimedia.org wrote: This is a situation where disciplined testing can come in really handy. If I submit a patch, and the patch passes the tests that have been specified for the feature it implements (or the bug it fixes), and the code coverage is sufficiently high, then a reviewer has a running start in terms of confidence in the correctness and completeness of the patch. Practically speaking, this doesn't necessarily rely on rest of the project already having a very level of code coverage; as long as there are tests laid out for the feature in question, and the patch makes those tests pass, it gives the code reviewer a real shot in the arm. On Thu, Jan 29, 2015 at 1:14 PM, Jon Robson jdlrob...@gmail.com wrote: Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. On Thu, Jan 29, 2015 at 1:11 PM, Brad Jorsch (Anomie) bjor...@wikimedia.org wrote: On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: The average time for code to go from submitted to merged appears to be 29 days over a dataset of 524 patches, excluding all that were written by the L10n bot. There is a patchset there that has been _open_ for 766 days - if you look at it it was uploaded on Dec 23, 2012 12:23 PM is -1ed by me and needs a rebase. Mean or median? I recall talk a while back about someone else (Quim, I think?) doing this same sort of analysis, and considering the same issues over patches that seem to have been abandoned by their author and so on, which led to discussions of whether we should go around abandoning patches that have been -1ed for a long time, etc. Without proper consideration of those sorts of issues, the statistics don't seem particularly useful. -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- Jon Robson * http://jonrobson.me.uk *
[Wikitech-l] Blog post on Librarization project published
For the last four months, my main focus has been the Librarization project [0]. Today a wrap up blog post was posted to blog.wikimedia.org [1] that I'd invite all of you to read to get an overview of what our high level goals and motivations were and what we accomplished. The TL;DR is that we now have some guidelines for how to separate code from MediaWiki and publish it as a semi-autonomous open source project. The blog post ends with a thinly veiled call to action for MediaWiki developers to continue the work of extracting code from the current MediaWiki core application and publishing them as independent libraries. We've published some information on how to deal with git hosting, code review, and various other general issues on mediawiki.org [2]. There is also a list of some areas of the existing code base that we thought would be interesting targets for extraction [3]. The CDB library [4] can serve as one concrete example of using the guidelines. I'd like to invite anyone interested in starting work on decoupling a particular area of the code to start a thread on wikitech-l and file a task in Librarization phabricator project [5] to attract collaborators and help reduce possible duplication of effort. It would also be great to have edits on the list page and/or phabricator tasks to act as a wish list of things that know of in MediaWiki that you would either like to be able to use in a non-MediaWiki PHP project or feel would be a good candidate for isolation so that alternate implementations could be introduced. [0]: https://www.mediawiki.org/wiki/Library_infrastructure_for_MediaWiki [1]: https://blog.wikimedia.org/2015/01/29/modernizing-mediawiki-with-libraries/ [2]: https://www.mediawiki.org/wiki/Manual:Developing_libraries [3]: https://www.mediawiki.org/wiki/Library_infrastructure_for_MediaWiki/Library_list [4]: https://www.mediawiki.org/wiki/CDB [5]: https://phabricator.wikimedia.org/tag/librarization/ Bryan -- Bryan Davis Wikimedia Foundationbd...@wikimedia.org [[m:User:BDavis_(WMF)]] Sr Software EngineerBoise, ID USA irc: bd808v:415.839.6885 x6855 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
On Thu, Jan 29, 2015 at 12:56 PM, Jon Robson jdlrob...@gmail.com wrote: Introducing: https://www.mediawiki.org/wiki/Extension_health Interesting! I'm hopping between flights back to Europe, and I don't have time to review these metrics more carefully, but please check http://korma.wmflabs.org/browser/gerrit_review_queue.html and let me know what you miss. -- Quim Gil Engineering Community Manager @ Wikimedia Foundation http://www.mediawiki.org/wiki/User:Qgil ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
Brion Vibber wrote: Good point Yuri -- a lot of those items on my queue are assigned to several reviewers so none of us feels ownership, and that's definitely part of the reason some of them sit around so long. A regular bot run that assigns untouched review requests to a single person in Phab probably does make sense... Many Gerrit changesets already have an associated Phabricator task, of course. I'm wary of trying to tear down one queue by building another. For Bugzilla (now Phabricator), we ended up creating a full-time position that focuses on reviewing, triaging, and generally handling bugs (now tasks). While I'm not still not sure I agree with this approach, a Review Wrangler or similar wouldn't be without precedent. MZMcBride ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Urlencoding strip markers
On Thu, Jan 29, 2015 at 2:47 PM, Arlo Breault abrea...@wikimedia.org wrote: There’s a brief discussions of the security implications of some proposed solutions in the review of, https://gerrit.wikimedia.org/r/#/c/181519/ To clarify, the possible solutions seem to be: 1. Unstrip the marker and then encode the content. This is a security hole (T73167) 2. Encode the marker. This results in strip markers in the output. 3. Ignore the marker. This leaves non-encoded content in the middle of what is supposed to be encoded content. 4. Remove the marker. This loses whatever is inside the marker. 5. Just output an error, to make it obvious something stupid is going on. There's no good option, so which of 2, 3, 4, and 5 is least bad? -- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] [Engineering] Blog post on Librarization project published
Great blog post and awesome achievements! I've spotted a small typo in the text: that offer more compelling featuresd. On Thu, Jan 29, 2015 at 5:01 PM, Bryan Davis bd...@wikimedia.org wrote: For the last four months, my main focus has been the Librarization project [0]. Today a wrap up blog post was posted to blog.wikimedia.org [1] that I'd invite all of you to read to get an overview of what our high level goals and motivations were and what we accomplished. The TL;DR is that we now have some guidelines for how to separate code from MediaWiki and publish it as a semi-autonomous open source project. The blog post ends with a thinly veiled call to action for MediaWiki developers to continue the work of extracting code from the current MediaWiki core application and publishing them as independent libraries. We've published some information on how to deal with git hosting, code review, and various other general issues on mediawiki.org [2]. There is also a list of some areas of the existing code base that we thought would be interesting targets for extraction [3]. The CDB library [4] can serve as one concrete example of using the guidelines. I'd like to invite anyone interested in starting work on decoupling a particular area of the code to start a thread on wikitech-l and file a task in Librarization phabricator project [5] to attract collaborators and help reduce possible duplication of effort. It would also be great to have edits on the list page and/or phabricator tasks to act as a wish list of things that know of in MediaWiki that you would either like to be able to use in a non-MediaWiki PHP project or feel would be a good candidate for isolation so that alternate implementations could be introduced. [0]: https://www.mediawiki.org/wiki/Library_infrastructure_for_MediaWiki [1]: https://blog.wikimedia.org/2015/01/29/modernizing-mediawiki-with-libraries/ [2]: https://www.mediawiki.org/wiki/Manual:Developing_libraries [3]: https://www.mediawiki.org/wiki/Library_infrastructure_for_MediaWiki/Library_list [4]: https://www.mediawiki.org/wiki/CDB [5]: https://phabricator.wikimedia.org/tag/librarization/ Bryan -- Bryan Davis Wikimedia Foundationbd...@wikimedia.org [[m:User:BDavis_(WMF)]] Sr Software EngineerBoise, ID USA irc: bd808v:415.839.6885 x6855 ___ Engineering mailing list engineer...@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/engineering ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] From Node.js to Go
Language fragmentation is always fun, but, as with any new one, my concerns lie in the environment - is there enough tools to make the advertised benefits worth it, does it have a decent IDE with the smart code completion, refactoring, and a good debugger? Does it have a packaging/dependency system? How extensive is the standard library, and user contributed packages. How well does it play with the code written in other languages? The list could go on. In short - we can always try new things as a small service )) And yes, Rust also sounds interesting. On Jan 29, 2015 7:22 PM, Ori Livneh o...@wikimedia.org wrote: (Sorry, this was meant for wikitech-l.) On Thu, Jan 29, 2015 at 7:20 PM, Ori Livneh o...@wikimedia.org wrote: We should do the same, IMO. http://bowery.io/posts/Nodejs-to-Golang-Bowery/ ___ 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] Improving our code review efficiency
On 2015-01-29 1:14 PM, Jon Robson wrote: Thanks for kicking off the conversation Brad :-) Just mean at the moment. I hacked together and I'm more than happy to iterate on this and improve the reporting. On the subject of patch abandonment: Personally I think we should be abandoning inactive patches. They cause unnecessary confusion to someone coming into a new extension wanting to help out. We may want to change the name to 'abandon' to 'remove from code review queue' as abandon sounds rather nasty and that's not at all what it actually does - any abandoned patch can be restored at any time if necessary. Unfortunately, under Gerrit, abandoning a patch puts inactive, restore if you can finish it patches in the same bin as this was a complete failure. Not only do you have to examine each abandoned patch individually to see if it's worth reopening, but after they leave your recently closed list they are all segregated to an obscure place not everyone knows how to get to (you have to manually search for owner:self status:abandoned). Proper handling of 'remove from code review queue' abandonment should include a section on a user's dashboard listing patches that have been removed from the queue due to inactivity, etc... but not outright rejected. ~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/] ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] From Node.js to Go
(Sorry, this was meant for wikitech-l.) On Thu, Jan 29, 2015 at 7:20 PM, Ori Livneh o...@wikimedia.org wrote: We should do the same, IMO. http://bowery.io/posts/Nodejs-to-Golang-Bowery/ ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] From Node.js to Go
I'm personally more excited about Rust. It is a true systems language with a modern type system, does away with the GC for more predictable performance and generally outperforms Go on CPU-bound tasks. It could actually become an interesting option for a highly parallel Parsoid 2.0 version once its 1.0 is out of the door. The Mozilla folks have built solid PEG and DOM libraries, which are important for that task. In any case, I see little benefit in porting existing Node code to Go right now. The performance gains are marginal, and the cost of language fragmentation is real. We have a large number of JS developers, and it looks like the web is not going to move away from JS any time soon. Modern JS with promises and generators is also quite a bit nicer than old-style callbacks, and we are seeing speed-ups of around 10% with io.js. Gabriel PS: Wikia are building an auth service in Go https://github.com/Wikia/helios/, but have otherwise standardized on Java for now. On Thu, Jan 29, 2015 at 7:21 PM, Ori Livneh o...@wikimedia.org wrote: (Sorry, this was meant for wikitech-l.) On Thu, Jan 29, 2015 at 7:20 PM, Ori Livneh o...@wikimedia.org wrote: We should do the same, IMO. http://bowery.io/posts/Nodejs-to-Golang-Bowery/ ___ 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] [Engineering] DevSummit appreciation
Huge +tons to everything below (and top posting to drive Mz up the wall) :-) As Erik said, I know I'm probably going to be driving Rachel and Quim nuts with my handwringing and second guessing as we figure out the pros and cons of how things went this year so that we can keep improving this. But, both of them (and everyone they pulled in) worked like hell put on a great show, and gave us a wonderful space to have some great conversations about our future engineering direction. Thank you! Rob On Wed, Jan 28, 2015 at 9:15 AM, Brion Vibber bvib...@wikimedia.org wrote: Agreed - we had a great space and good support, the WiFi worked, power strips everywhere, and there was always coffee. I can ask for little more... ;) Thanks also to our fellow attendees -- I had a lot of great conversations and got a lot of data points to help set my work directions for the coming months. Everybody there was awesome even when we had contentious issues -- I want to thank everybody for having a positive attitude and working together. -- brion On Jan 27, 2015 10:44 PM, Erik Moeller e...@wikimedia.org wrote: Just a quick note that I really appreciated everyone's help making the summit come together. As always, we'll be doing lots of second-guessing of everything we did and didn't do, and how we want to use future time together. Before we go into that, I'd like to thank the event team and _everyone_ who worked to and beyond the point of exhaustion to organize the event, support attendees, plan sessions, facilitate conversations, negotiate sometimes difficult terrain. Thank you. :) Erik -- Erik Möller VP of Product Strategy, Wikimedia Foundation ___ Engineering mailing list engineer...@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/engineering ___ 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
[Wikitech-l] Converting debug logging to PSR-3
PSR-3 logging has been fully supported in MediaWiki since1.25wmf5. We've been making various tuning improvements since then including a recent deprecation of the initial MWLogger wrapper class in favor of direct usage of Psr\Log\LoggerInterface by wfDebugLog() and other internal wrapper methods [2]. The next big step in introducing structured logging to MediaWiki is to begin to replace calls to wfDebugLog() (and dear god wfDebug()) with direct usage of Psr\Log\LoggerInterface instances. Kunal and I have a couple starter patches that show this in gerrit [3]. In both conversions we have chosen to implement Psr\Log\LoggerAwareInterface in the effected classes to allow setter based injection of a LoggerInterface. The log channel names were also chosen to match the previous wfDebugLog logGroup values. When you use a PSR-3 logger you need to choose a severity for each log message. PSR-3 has quite a few possible levels, but I'd like to propose that we really only need 4 to start with in MediaWiki: * debug: Useful for ummm debugging. :) These are messages that are useful for local development and are generally too spammy to output on a production wiki. This would typically include anything currently being logged via wfDebug. * info: Valuable state change information. This level is a great place to record information that would be useful in a production environment when tracing the path of a request that eventually had an error. * warning: A soft error condition such as a recoverable error or another condition that typically should not be seen but isn't halting for the operation in process * error: A hard error such as a caught exception with no recovery path. The notice, critical, alert and emergency log levels seem unnecessary to me, but I'm willing to hear arguments about where they are super duper useful for some log event state that I haven't thought of yet. When thinking about Wikimedia cluster logging, events emitted at warning and error levels should be things that you want deployers and operations staff to see in the Logstash fatalmonitor view and recorded to other durable logging stores. Events at info level may or may not be captured similar to how we currently enable some but not all wfDebugLog channels. Events at debug level will probably only be captured in beta labs and similar low volume debugging environments. The wfDebug* methods are not being deprecated officially yet but it would be great if people started treating them like they were deprecated when writing new classes. It would be even more awesome if more folks started making small cleanup patches to convert existing classes to the new style of logging. Tagging gerrit reviews for these patches with PSR-3 as either a submitter or a reviewer would also be appreciated. [0]: https://gerrit.wikimedia.org/r/#/c/119940/ [1]: https://gerrit.wikimedia.org/r/#/c/185210/ [2]: https://gerrit.wikimedia.org/r/#/c/184830/ [3]: https://gerrit.wikimedia.org/r/#/q/project:mediawiki/core+branch:master+topic:PSR-3,n,z Bryan -- Bryan Davis Wikimedia Foundationbd...@wikimedia.org [[m:User:BDavis_(WMF)]] Sr Software EngineerBoise, ID USA irc: bd808v:415.839.6885 x6855 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] Improving our code review efficiency
The remove from code review queue is not that hard really, you can just remove yourself (and reviewers you added) from reviewers. The most helpful reviewers also comment on why they've removed themselves. If reviewers removed themselves from patches they have no intention whatsoever to review (which is fine), things would be much easier. Other effective ways to effectively remove an open patch from the review workflows (and from the korma stats, though not from [[Gerrit/Reports]]) is to self-give a -1 and/or slap a [WIP] in the first line. Nemo ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
