On Friday, April 4, 2014, Happy Melon happy.melon.wiki@gmail.com wrote:
On 4 April 2014 00:38, Brian Wolff <bawolff@gmail.com javascript:;> wrote:
what about directing new volunteers there, asking them to submit their
code
revisions. For a patch that has been waiting in silence for over a
year,
any feedback will be better than no feedback.
You sure about that? I would imagine that having no one look at your code for months, then having someone who doesn't have the authority to approve it nit pick it a little, followed by another couple months of waiting, to be more frustrating then no feedback at all.
No, I'm not sure. Now those changesets are buried somewhere. The hypothesis is that channeling some attention to them cannot make things worse. Perhaps the nitpicking helps to bring back the attention of the maintainers?
Maybe we can dig deeper in the metrics, and find changesets with last versions of patches waiting for a first review, a different case from, say, open changesets with many versions, many comments and some +1 (a place where a newcomer would not add much).
This is also completely the wrong way to go about open-source development. The work priorities of volunteers are the thing that you, as manager of paid staff, *can't* control, as opposed to the work priorities of paid staff, which you very much can.
Agreed, and one of the reasons for producing these metrics is to help paid developers prioritize their work taking into account the task of reviewing the contributions they receive. However...
If reviewing these old patches was in any way interesting/exciting/fulfilling, volunteers would probably already have *made* some contributions.
This would be true if such potential contributions could be easily found by new contributors. Currently they are well hidden, you really need to know where to go in order to find a Gerrit changeset welcoming feedback.
Can we define URLs to Gerrit queries to be advertized at https://www.mediawiki.org/wiki/Annoying_little_bugs ? For instance, we offer links to EASY bugs, and looking at the paths chosen by e.g. potential GSoC candidates I would say that the system kind of works.
Being occasionally tasked with uninteresting/unexciting/unfulfilling tasks that Just Need Doing is one of the things that paid developers *get *paid *for*.
Many tasks that are considered uninteresting/unexciting/unfulfilling by experienced contributors are interesting/exciting/fulfilling for new contributors. Again, the EASY bugs are a good example. Ideas to define good tasks for newcomers in our review queues are welcome.
No, I'm not sure. Now those changesets are buried somewhere. The
hypothesis
is that channeling some attention to them cannot make things worse.
Perhaps
the nitpicking helps to bring back the attention of the maintainers?
Ways it could make things worse: *incorrect advice given (if the newbies knew what to look for in a patch, by definition i wouldnt call them a newbie). This is a real risk if we start directing inexperianced people to do code review. Patch contributors are often new people too, leading them astray has a big cost *giving people false hope that their patch will get metged and in the end making them more fet up with the entire process.
If a newbie tests a patch and then says something like - this fixes the problem for me, that is mildly helpful. I worry if we start encouraging newbies to do CR from the how to become a mw hacker page, we will end up with silly comments like "-1: missing trailing ?>".
Maybe we can dig deeper in the metrics, and find changesets with last versions of patches waiting for a first review, a different case from,
say,
open changesets with many versions, many comments and some +1 (a place where a newcomer would not add much).
Most patches are like the former. Its relatively rare to have a patch with many versions and a couple +1's unless its actively in the process of being reviewed or pending some external event.
Many tasks that are considered uninteresting/unexciting/unfulfilling by experienced contributors are interesting/exciting/fulfilling for new contributors. Again, the EASY bugs are a good example. Ideas to define
good
tasks for newcomers in our review queues are welcome.
Sure, but its kind of an oxymoron to say, hey people who are new and unfamilar with our social and technical conventions, please asses wether this patch meets those social and technical conventions.
-- Bawolff
Quim Gil qgil@wikimedia.org wrote:
[...]
This is also completely the wrong way to go about open-source development. The work priorities of volunteers are the thing that you, as manager of paid staff, *can't* control, as opposed to the work priorities of paid staff, which you very much can.
Agreed, and one of the reasons for producing these metrics is to help paid developers prioritize their work taking into account the task of reviewing the contributions they receive. However...
[...]
This sounds like a guide to procrastination. "There was so much work to choose from, boss, so I did none."
I'm still (a bit) interested in the effect of all this man- agement on improving the process (cf. http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/544...), but if the issue at hand is to review the ~ 2500 "code re- views waiting for reviewer", if every one of the ~ 100 WMF employees reviews one (additional) changeset per workday, the task will be done by the end of this month.
There is already https://gerrit.wikimedia.org, "git review -l" and Jon's (brilliant) tool (cf. https://raw.github.com/jdlrobson/GerritCommandLine/master/gerrit.py) to pick a change to review; and the nice thing about reviewing all of them in a foreseeable time period is that you don't have to prioritize! Just pick any!
Just in case it got lost in transmission: Pick any!
Tim
Alright, let's not attempt to mix new contributors and code review, then. Thank you for your good arguments.
On Friday, April 4, 2014, Tim Landscheidt tim@tim-landscheidt.de wrote:
if the issue at hand is to review the ~ 2500 "code re- views waiting for reviewer", if every one of the ~ 100 WMF employees reviews one (additional) changeset per workday, the task will be done by the end of this month.
As far as I can see, there are not 100 WMF employees capable of reviewing changesets, even less of merging them. Not only this, expertise and +2 rights must be seen from the point of view of repositories, because not every reviewer can review every patch. Once we solve "Bug 63533 - Gerrit metrics about open changesets should ignore -1s", we will be able to see what kind of problem we have, and that will help us decide what needs to be done.
There is already https://gerrit.wikimedia.org, "git review -l" and Jon's (brilliant) tool (cf. https://raw.github.com/jdlrobson/GerritCommandLine/master/gerrit.py) to pick a change to review;
Brilliant indeed! Whatever we do, it is going to be useful (friendlier URL: https://github.com/jdlrobson/GerritCommandLine )
and the nice thing about reviewing all of them in a foreseeable time period is that you don't have to prioritize! Just pick any!
I reckon your strategy is sexy. More when we have reliable numbers.
Just in case it got lost in transmission: Pick any!
+1
On 4/5/14, Quim Gil qgil@wikimedia.org wrote:
Alright, let's not attempt to mix new contributors and code review, then. Thank you for your good arguments.
On Friday, April 4, 2014, Tim Landscheidt tim@tim-landscheidt.de wrote:
if the issue at hand is to review the ~ 2500 "code re- views waiting for reviewer", if every one of the ~ 100 WMF employees reviews one (additional) changeset per workday, the task will be done by the end of this month.
As far as I can see, there are not 100 WMF employees capable of reviewing changesets, even less of merging them. Not only this, expertise and +2 rights must be seen from the point of view of repositories, because not every reviewer can review every patch. Once we solve "Bug 63533 - Gerrit metrics about open changesets should ignore -1s", we will be able to see what kind of problem we have, and that will help us decide what needs to be done.
Indeed. as it stands, there are only 78 people who have ever +2'd even a single changeset in mediawiki/core, of which only 53 are staff. So if 100 WMF employees reviewed a change set today, for roughly half of them it would be the first time they had ever done so (I've excluded extensions from this analysis. Also I counted quickly, don't use those numbers for anything important).
Of course, by all means if you're a WMF employee who is knowledgeable enough to review something (Or if you are a volunteer that has +2 rights and knowledgeable enough), there's no better time to start reviewing things then today :)
-- bawolff
Brian Wolff bawolff@gmail.com wrote:
Alright, let's not attempt to mix new contributors and code review, then. Thank you for your good arguments.
if the issue at hand is to review the ~ 2500 "code re- views waiting for reviewer", if every one of the ~ 100 WMF employees reviews one (additional) changeset per workday, the task will be done by the end of this month.
As far as I can see, there are not 100 WMF employees capable of reviewing changesets, even less of merging them. Not only this, expertise and +2 rights must be seen from the point of view of repositories, because not every reviewer can review every patch. Once we solve "Bug 63533 - Gerrit metrics about open changesets should ignore -1s", we will be able to see what kind of problem we have, and that will help us decide what needs to be done.
Indeed. as it stands, there are only 78 people who have ever +2'd even a single changeset in mediawiki/core, of which only 53 are staff. So if 100 WMF employees reviewed a change set today, for roughly half of them it would be the first time they had ever done so (I've excluded extensions from this analysis. Also I counted quickly, don't use those numbers for anything important).
Of course, by all means if you're a WMF employee who is knowledgeable enough to review something (Or if you are a volunteer that has +2 rights and knowledgeable enough), there's no better time to start reviewing things then today :)
This implies that the changesets waiting for a reviewer only need someone to +2 them in the condition that they are in at the moment. I'd guess (no data to back it up! :-)) this is not correct for the majority of changesets that will need several iterations to incorporate (valid) suggestions from the review.
Tim
We are using a pre-review hook in MobileFrontend now. It stops us submitting new patches when there is existing code to be reviewed.
If interested see https://www.mail-archive.com/mobile-l@lists.wikimedia.org/msg01283.html On 5 Apr 2014 11:51, "Tim Landscheidt" tim@tim-landscheidt.de wrote:
Brian Wolff bawolff@gmail.com wrote:
Alright, let's not attempt to mix new contributors and code review,
then.
Thank you for your good arguments.
if the issue at hand is to review the ~ 2500 "code re- views waiting for reviewer", if every one of the ~ 100 WMF employees reviews one (additional) changeset per workday, the task will be done by the end of this month.
As far as I can see, there are not 100 WMF employees capable of
reviewing
changesets, even less of merging them. Not only this, expertise and +2 rights must be seen from the point of view of repositories, because not every reviewer can review every patch. Once we solve "Bug 63533 - Gerrit metrics about open changesets should ignore -1s", we will be able to see what kind of problem we have, and that will help us decide what needs
to be
done.
Indeed. as it stands, there are only 78 people who have ever +2'd even a single changeset in mediawiki/core, of which only 53 are staff. So if 100 WMF employees reviewed a change set today, for roughly half of them it would be the first time they had ever done so (I've excluded extensions from this analysis. Also I counted quickly, don't use those numbers for anything important).
Of course, by all means if you're a WMF employee who is knowledgeable enough to review something (Or if you are a volunteer that has +2 rights and knowledgeable enough), there's no better time to start reviewing things then today :)
This implies that the changesets waiting for a reviewer only need someone to +2 them in the condition that they are in at the moment. I'd guess (no data to back it up! :-)) this is not correct for the majority of changesets that will need several iterations to incorporate (valid) suggestions from the review.
Tim
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Le 05/04/2014 20:23, Brian Wolff a écrit :
Of course, by all means if you're a WMF employee who is knowledgeable enough to review something (Or if you are a volunteer that has +2 rights and knowledgeable enough), there's no better time to start reviewing things then today :)
I am myself no more reviewing changes in core unless being asked. I am more or less off the PHP swamp and headed toward the puppet/python cliffs.
wikitech-l@lists.wikimedia.org