-- tl;dr start -- We're using gerrit now. And as we use it we all keep running into issues stemming fundamentally from how gerrit is designed.
I had an irc discussion with Roan[1] awhile ago tossing around ideas on how we would write a review system for git if we wrote one from scratch. Based on a number of things I noted there I started a draft of how such a review system would work: https://www.mediawiki.org/wiki/User:Dantman/CodeReviewSystem
I probably haven't written down nearly as much detail on that page as I've thought out for the actual system.
This is a long term project. It's definitely going to take awhile before it's ready to manage MediaWiki. However we're using git now, so this project doesn't have as much pressure. Since git is distributed things can easily coexist; We can put this up on labs before it's ready for primetime. When it's somewhat ready we can opt-in to trying it on less important repos. And even when we switch it will be possible to do so in a way such that gerrit and the review system co-exist until all changesets in gerrit have finished being reviewed. -- tl;dr end --
In one of my one-day bouts of coding I started trying to implement the system: https://github.com/dantman/gareth
Right now the code is EXTREMELY rough, and it barely does anything. My focus is on getting the fundamentals needed for the system to work (otherwise it's a fancy ui that does nothing). So I've made short-cuts on a lot of things so I don't get held back implementing the clean stuff.
I'm probably the only person with a whole enough idea of how this'll work to push forward on the fundamental parts of it. However despite that I welcome help on the project and have a number of things that other people should still be able to write while I work on the fundamental pieces.
Right now I've skipped past a number of things: - We need a views system, something that will let us template out many sections of html. And will allow different themes to override them. - Right now I've copied some of the classes from MediaWiki like WebRequest and stripped stuff out of them. Anyone up to it is free to clean these up into the start of a real framework based on MW's code that can in the future be the base of both this and MW. - The project needs a good database system. I copied our database classes in but never got to using them. I'm isolating all database stuff into some model classes so different database handing can be swapped in. Anyone who feels up to it can adapt our database code to work as a framework for the review system. - Right now I'm implementing git handling using proc_open to interact with git's porcelain and plumbing commands. Anyone who feels up to the task is free to implement a PHP extension for interfacing with git we can swap over to using. - I'd really like to eventually have a way to list a set of commits using a nice line graph the way that things like gitk, gitx, and source tree too. I welcome anyone who feels like trying to write something like a JS library to generate in-page SVG (Raphael?) making a vertical list of commits listed that way.
I welcome anyone who feels like turning the short-cuts into a properly written system. And anyone who feels like turning MediaWiki's code into a framework. Or anyone who feels that a framework they know fits best and wants to make the review system use it (Though no file layout controlling MVC frameworks, please).
[1]: The discussion was buried somewhere deep in http://bots.wmflabs.org/~petrb/logs/%23mediawiki/20120329.txt try somewhere around [20:16:09]
Le 08/04/12 14:44, Daniel Friesen a écrit :
anyone who feels that a framework they know fits best and wants to make the review system use it (Though no file layout controlling MVC frameworks, please).
For a PHP framework, have a look at Symfony: http://www.symfony-project.org/
If I wanted to write a web application from scratch, I would use JavaScript with node.js.
Le 08/04/12 14:44, Daniel Friesen wrote :
We're using gerrit now. And as we use it we all keep running into issues stemming fundamentally from how gerrit is designed.
<snip>
Hello Daniel,
We already had the experience of writing a code review tool from scratch and although it filled a need at that time, I am not going to regret it.
I do not think the foundation should fund the project of writing a new app from scratch which probably nobody else will use anyway and based on a niche Framework which is MediaWiki. Instead we should stick to reuses existing tools and have them enhanced to fit our needs, writing a whole new app should be a solution of last resort.
Before starting to write code in a rush, your really first step should be to find out what is *functionally* wrong with Gerrit and find out what we need. I have yet to find out anyone describing what is wrong with Gerrit although we all have a sentiment that the UI is confusing and some obvious technical issues. They do not justify a rewrite in my opinion.
Once you have a list of requirements, though, we could look at the possibility of fixing Gerrit to fit our needs. If that is not possible, we could use that list as a basis to find out another tool. You could coordinate with John Du Hart who has been working on Phabricator, a suite of tools from Facebook which include a code review tool as I understand it. Other examples that comes to mind are GitHub and Gitorious.
Meanwhile, dont waste your time coding something we are most certainly never going to use.
cheers,
On Sun, Apr 8, 2012 at 4:18 PM, Antoine Musso hashar+wmf@free.fr wrote:
Meanwhile, dont waste your time coding something we are most certainly never going to use.
Statements like that from WMF staff are hurtful towards volunteer developers. Don't turn this into another MobileFrontend situation.
On Sun, Apr 8, 2012 at 7:26 PM, John Du Hart compwhizii@gmail.com wrote:
Meanwhile, dont waste your time coding something we are most certainly never going to use.
Statements like that from WMF staff are hurtful towards volunteer developers. Don't turn this into another MobileFrontend situation.
Antoine's welcome to speak his mind freely, like anyone; there's no party line. I agree with him that we're unlikely to invest any significant effort in this unless it gains substantial traction and starts to look like a real alternative. But I don't think Daniel was asking for that -- he wants to give it a go, and get others excited about it. Which is awesome, and exactly what Labs was made for. :-)
WMF is likely going to focus its efforts on participating more in the gerrit development process and evaluating existing alternatives.
Antoine's welcome to speak his mind freely, like anyone; there's no party line. I agree with him that we're unlikely to invest any significant effort in this unless it gains substantial traction and starts to look like a real alternative. But I don't think Daniel was asking for that -- he wants to give it a go, and get others excited about it. Which is awesome, and exactly what Labs was made for. :-)
+1. I'm more than happy to make a labs project for this. In fact, I just created it.
- Ryan
You're a brave soul! Can we get a labs project for testing that in real time?
I'm not sure about the personal public remotes. If we're going to continue with our way of contribution, we would have continuous bases. OTOH it looks cool to be able to work A1--A2--B1--B2 without creating new branches at each step.
The review-heads stuffs is perhaps too complicated. I think it'd be simpler to base in the well-known monotonic-increased ids. So the review-queue item 1024 is at refs/review-heads/1024 where you pull the latest item (or the full set?), and each forked head for that review has refs/review-heads/1024/1, refs/review-heads/1024/2... Plus the aliasing of 1024 to Voice-to-wikitext-feature.
Reuse of old names might be confusing. Maybe if a name is reused it should get named Feature_v2 and the original one automatically named Feature_v1. The mere presence of v2 indicates that there was a prior one.
- The project needs a good database system. I copied our database
classes in but never got to using them. I'm isolating all database stuff into some model classes so different database handing can be swapped in. Anyone who feels up to it can adapt our database code to work as a framework for the review system.
I think it would be easier with our classes. Too much fetch()s in ProjectModel.php
- Right now I'm implementing git handling using proc_open to interact
with git's porcelain and plumbing commands. Anyone who feels up to the task is free to implement a PHP extension for interfacing with git we can swap over to using.
Please, remove the usage of __call() there. Make a different function for each one, even if they're going to be dummy ones right now. This way we can easily replace them with real implementations instead of wondering which ones are called by the magic.
On Sun, 08 Apr 2012 13:32:07 -0700, Platonides Platonides@gmail.com wrote:
You're a brave soul! Can we get a labs project for testing that in real time?
I'm not sure about the personal public remotes. If we're going to continue with our way of contribution, we would have continuous bases. OTOH it looks cool to be able to work A1--A2--B1--B2 without creating new branches at each step.
Well, you can work that way. Though I think gareth will still have similar handling where using a branch name makes the review system automatically guess what name to use for the topic/review branch.
It also shields you from some unnecessary merge conflicts.
The review-heads stuffs is perhaps too complicated. I think it'd be simpler to base in the well-known monotonic-increased ids. So the review-queue item 1024 is at refs/review-heads/1024 where you pull the latest item (or the full set?), and each forked head for that review has refs/review-heads/1024/1, refs/review-heads/1024/2... Plus the aliasing of 1024 to Voice-to-wikitext-feature.
Reuse of old names might be confusing. Maybe if a name is reused it should get named Feature_v2 and the original one automatically named Feature_v1. The mere presence of v2 indicates that there was a prior one.
Hmm... I'll consider it. But this project's focus has been trying to embrace the git way of doing things; since some of gerrit's issues stem from ignoring that way of doing things.
- The project needs a good database system. I copied our database
classes in but never got to using them. I'm isolating all database stuff into some model classes so different database handing can be swapped in. Anyone who feels up to it can adapt our database code to work as a framework for the review system.
I think it would be easier with our classes. Too much fetch()s in ProjectModel.php
Someone would need to adapt the classes to work outside MW. I started using Mysqli and prepared statements since it's an easy way to get the database stuff out of the way right away.
- Right now I'm implementing git handling using proc_open to interact
with git's porcelain and plumbing commands. Anyone who feels up to the task is free to implement a PHP extension for interfacing with git we can swap over to using.
Please, remove the usage of __call() there. Make a different function for each one, even if they're going to be dummy ones right now. This way we can easily replace them with real implementations instead of wondering which ones are called by the magic.
Drop ShellGit entirely and make Git the only class?
On 08/04/12 22:49, Daniel Friesen wrote:
- The project needs a good database system. I copied our database
classes in but never got to using them. I'm isolating all database stuff into some model classes so different database handing can be swapped in. Anyone who feels up to it can adapt our database code to work as a framework for the review system.
I think it would be easier with our classes. Too much fetch()s in ProjectModel.php
Someone would need to adapt the classes to work outside MW. I started using Mysqli and prepared statements since it's an easy way to get the database stuff out of the way right away.
I think they work. You do:
$conf = array( 'host' => ..., 'user' => ..., 'pass'=> ..., 'dbname' =>..., 'tableprefix'=> ... ) $db = Database::factory( 'mysql', $conf );
And you have a perfectly working Database object.
Or if you prefer something much more lightweight, https://fisheye.toolserver.org/browse/erfgoed/api/includes/Database.php?hb=t... is a Database class which aims to provide a similar interface to ours.
- Right now I'm implementing git handling using proc_open to interact
with git's porcelain and plumbing commands. Anyone who feels up to the task is free to implement a PHP extension for interfacing with git we can swap over to using.
Please, remove the usage of __call() there. Make a different function for each one, even if they're going to be dummy ones right now. This way we can easily replace them with real implementations instead of wondering which ones are called by the magic.
Drop ShellGit entirely and make Git the only class?
No, I mean: function diff() { return call_user_func_array( array( $this->git, __METHOD__ ), get_func_args() ); }
function show() { return call_user_func_array( array( $this->git, __METHOD__ ), get_func_args() ); }
etc.
(still, I think ShellGit misses a __call and won't work right now)
So what happened to considering Phabricator in the future?
On Sun, Apr 8, 2012 at 5:29 PM, Platonides Platonides@gmail.com wrote:
On 08/04/12 22:49, Daniel Friesen wrote:
- The project needs a good database system. I copied our database
classes in but never got to using them. I'm isolating all database stuff into some model classes so different database handing can be swapped in. Anyone who feels up to it can adapt our database code to work as a framework for the review system.
I think it would be easier with our classes. Too much fetch()s in ProjectModel.php
Someone would need to adapt the classes to work outside MW. I started using Mysqli and prepared statements since it's an easy way to get the database stuff out of the way right away.
I think they work. You do:
$conf = array( 'host' => ..., 'user' => ..., 'pass'=> ..., 'dbname' =>..., 'tableprefix'=> ... ) $db = Database::factory( 'mysql', $conf );
And you have a perfectly working Database object.
Or if you prefer something much more lightweight, https://fisheye.toolserver.org/browse/erfgoed/api/includes/Database.php?hb=t... is a Database class which aims to provide a similar interface to ours.
- Right now I'm implementing git handling using proc_open to interact
with git's porcelain and plumbing commands. Anyone who feels up to the task is free to implement a PHP extension for interfacing with git we can swap over to using.
Please, remove the usage of __call() there. Make a different function for each one, even if they're going to be dummy ones right now. This way we can easily replace them with real implementations instead of wondering which ones are called by the magic.
Drop ShellGit entirely and make Git the only class?
No, I mean: function diff() { return call_user_func_array( array( $this->git, __METHOD__ ), get_func_args() ); }
function show() { return call_user_func_array( array( $this->git, __METHOD__ ), get_func_args() ); }
etc.
(still, I think ShellGit misses a __call and won't work right now)
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 04/08/2012 10:23 PM, John Du Hart wrote:
So what happened to considering Phabricator in the future?
That evaluation is going to happen in June. It's on the roadmap for us to lead then: https://www.mediawiki.org/wiki/Roadmap#Platform
On Sun, Apr 8, 2012 at 5:44 AM, Daniel Friesen lists@nadir-seen-fire.comwrote:
-- tl;dr start -- We're using gerrit now. And as we use it we all keep running into issues stemming fundamentally from how gerrit is designed.
I had an irc discussion with Roan[1] awhile ago tossing around ideas on how we would write a review system for git if we wrote one from scratch. Based on a number of things I noted there I started a draft of how such a review system would work: https://www.mediawiki.org/**wiki/User:Dantman/**CodeReviewSystemhttps://www.mediawiki.org/wiki/User:Dantman/CodeReviewSystem
I probably haven't written down nearly as much detail on that page as I've
thought out for the actual system.
My views have evolved a bit on this, and I now believe a much more
feasible thing to do would be to use an existing project (gitosis or gitolite is probably best) for the repository/permissions management, and build a review system on top of that. As for the rest of the notes, I'll have to read those more thoroughly when I'm more capable of deep thought (i.e. not battling a cold).
This is a long term project. It's definitely going to take awhile before it's ready to manage MediaWiki. However we're using git now, so this project doesn't have as much pressure. Since git is distributed things can easily coexist; We can put this up on labs before it's ready for primetime. When it's somewhat ready we can opt-in to trying it on less important repos. And even when we switch it will be possible to do so in a way such that gerrit and the review system co-exist until all changesets in gerrit have finished being reviewed. -- tl;dr end --
In one of my one-day bouts of coding I started trying to implement the system: https://github.com/dantman/**gareth https://github.com/dantman/gareth
Love the name :) Will look at this once I feel better.
I welcome anyone who feels like turning the short-cuts into a properly written system. And anyone who feels like turning MediaWiki's code into a framework. Or anyone who feels that a framework they know fits best and wants to make the review system use it (Though no file layout controlling MVC frameworks, please).
I would love to help you with this once I have time to do so. Right now
I'm still getting settled after moving between countries, so essentially I have no free time. And I've been feeling sickly since Friday.
Roan
wikitech-l@lists.wikimedia.org