Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Amir Sarabadani]

Hello,
This is Amir, Chair of the code of conduct committee, talking on behalf of
it.

We received several reports about the discussion happening here which we
will handle and act accordingly but in the mean time, we would like to
remind everyone to follow CoC [1] and be more mindful of what they write.
Think with yourself when writing an email "Is this email moving the
discussion forward or it's just criticizing non-constructively?" If the
answer is the latter, please refrain from sending it. Insisting on sending
nonconstructive angry emails will get you banned temporarily or permanently
from this mailing list.

[1]: https://www.mediawiki.org/wiki/Code_of_Conduct

Regards,
Code of Conduct Committee, MediaWiki.

On Tue, Jan 22, 2019 at 4:36 PM Tyler Cipriani 
wrote:

> Hi all!
>
> This plugin has been removed entirely from Wikimedia Gerrit[0]. I know
> of no one who intended to experiment with the plugin in its current form
> so it is now removed.
>
> I have created a task to track suggestions for this plugin's
> improvement[1]. This task's scope is to track suggestions for
> improvements to the reviewers-by-blame plugin so they are not lost in
> this thread. Implementation details about individual suggestions should
> (likely) become seperate tasks.
>
> Any discussion about what is needed to re-enable this plugin for
> Wikimedia's Gerrit is a different discussion for another task and time.
> We won't re-enable this plugin without notice or without further
> discussion.
>
> >On 19-01-21 18:20:13, Paladox via Wikitech-l wrote:
> >FYI i have a working prototype working ("Suggest Reviewer") button.
> >
> >>On Monday, 21 January 2019, 16:32:35 GMT, Paladox via Wikitech-l <
> wikitech-l@lists.wikimedia.org> wrote:
> >>
> >>I’m currently working on addressing all the feedback as fast as I can.
> >>I honestly think this extension is great especially for new users, who
> >>do not know they need reviewers or who would review there change.
> >>Granted this extension has some problems hence why feature requests
> >>were filed against the extension.
>
> +1 to what Niharika and other have said: thank you for all your work on
> Gerrit, Paladox!
>
> You have made maintaining and keeping Gerrit secure easier for me
> personally.
>
> Thanks all for your thoughts on this thread, and thank you in advance for
> ensuring the task for suggestions for improvement is accurate.
>
> -- Tyler
>
> [0]. 
> [1]. 
> ___
> Wikitech-l mailing list
> Wikitech-l@lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l



-- 
Amir
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Tyler Cipriani]

Hi all!

This plugin has been removed entirely from Wikimedia Gerrit[0]. I know 
of no one who intended to experiment with the plugin in its current form 
so it is now removed.


I have created a task to track suggestions for this plugin's 
improvement[1]. This task's scope is to track suggestions for 
improvements to the reviewers-by-blame plugin so they are not lost in 
this thread. Implementation details about individual suggestions should 
(likely) become seperate tasks.


Any discussion about what is needed to re-enable this plugin for 
Wikimedia's Gerrit is a different discussion for another task and time. 
We won't re-enable this plugin without notice or without further 
discussion.



On 19-01-21 18:20:13, Paladox via Wikitech-l wrote:
FYI i have a working prototype working ("Suggest Reviewer") button.


On Monday, 21 January 2019, 16:32:35 GMT, Paladox via Wikitech-l 
 wrote:

I’m currently working on addressing all the feedback as fast as I can.
I honestly think this extension is great especially for new users, who
do not know they need reviewers or who would review there change.
Granted this extension has some problems hence why feature requests
were filed against the extension.


+1 to what Niharika and other have said: thank you for all your work on 
Gerrit, Paladox!


You have made maintaining and keeping Gerrit secure easier for me 
personally.


Thanks all for your thoughts on this thread, and thank you in advance for 
ensuring the task for suggestions for improvement is accurate.


-- Tyler

[0]. 
[1]. 


signature.asc
Description: PGP signature
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Antoine Musso]

On 18/01/2019 23:12, Pine W wrote:

I'm glad that this problematic change to communications was reverted.

I would like to suggest that this is the type of change that, when being
planned, should get a design review from a third party before coding
starts, should go through at least one RFC before coding starts, and be
widely communicated before coding starts and again a week or two before
deployment. Involving TechCom might also be appropriate. It appears that
none of those happened here. In terms of process this situation looks to me
like it's inexcusable.

In the English Wikipedia community, doing something like this would have a
reasonable likelihood of costing an administrator their tools, and I hope
that a similar degree of accountability is enforced in the engineering
community. In particular, I expect engineering supervisors to follow
established technical processes for changes that impact others' workflows,
and if they decide to skip those processes without a compelling reason
(such as a site stability problem) then I hope that they will be held
accountable. Again, from my perspective, the failure to follow process here
is inexcusable.

Pine


Hello Pine

We had the Gerrit reviewers-by-blame installed a while ago although it 
was not functional. Tyler, Gergo and I have been talking about that idea 
for quite a while and felt like it was a good idea to get patches reviewed.


On a quick though, if one authored some code, most probably that person 
knows the code and thus would qualify as a reviewers. For the few code I 
wrote from scratch, I am certainly interested in being added automatically.


Anyway, we went with upgrading the Gerrit plugin. I even wrote a blog 
post to explain a bit of the feature and other ways to find reviewers:

https://phabricator.wikimedia.org/phame/post/view/139/

I don't write blogs that often. If I do it is because I am excited about 
some feature which I firmly believe to be a general improvement.  I have 
been naive?  Yes surely.  Did we miss evaluating potential side effects? 
 For sure.


Then one as to take in perspective the cost of trying and reverting 
versus spending ages and months on a project only to dish it out because 
that is not what the customer wanted.  I, and several, choose the first 
path: quick cheap experiment with limited casualties. A benefit is that 
this thread gave a lot of exposure to the feature and gathered a lot of 
constructive feedback.  One can then easily conclude that the plugin is 
not smart enough and fails to spot the proper reviewers.


The plugin does not add reviewers any more (it defaults to add 0 
reviewers), and I guess we will just uninstall it entirely.



As for new features for Gerrit or Phabricator or CI. There is no due 
process established. We just f***ing do it, given we promptly rollback 
when we screw up which is thankfully rather rare.  Else we iterate and 
refine the feature until it is deemed stable.


That is how we maintain our infrastructure, not by having four hours 
meetings week after weeks with no deployment in between, not by having 
cross teams agreement, nor five level of political hierarchy drama.  We 
certainly had a few outages here and there, but given the very few 
people working on those parts and the number of modifications we do on a 
weekly basis, I think it is overall rather stable.



I am not willing to start a flame war, but I do not think the English 
Wikipedia community is a good example of an healthy one. The huge amount 
of process and policies makes it a challenge to have edits retained, it 
is partly what made me stop editing entirely.



In short, Pine, please assume good faith 8-]


--
Antoine "hashar" Musso
{^-^}


___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Cormac Parle]

+1 to Niharika - the initial iteration caused some inconvenience, but I expect 
subsequent iterations to be useful. Thank you Paladox!





> On 22 Jan 2019, at 13:09, Niharika Kohli  wrote:
> 
> On Tue, Jan 22, 2019 at 6:12 PM Paladox via Wikitech-l <
> wikitech-l@lists.wikimedia.org> wrote:
> 
>> What your saying is making me think I’m wasting my time on improving this
>> extension.
>> Also other users that have spoken to me have thought this extension is
>> great but could do with improvements which I am doing. We need to think of
>> new users and how to improve there experence. The task was opened for a
>> long while yet no one commented on it.
>> I agree with legoktm feedback.
>> “A process that annoys people based on nothing but the fact that
>> theyhappened to be the last one touching a file *is* fundamentally broken.”
>> yes hence why I’ve been making improvements by adding a button which is
>> better then nothing right?
>> As chad mentions it has no idea what is a typo fix compared to other
>> things as it’s not A.I.
>> 
> 
> Thanks for working on this, Paladox. I think this can be a really useful
> feature for newcomers and experienced developers alike, if implemented
> well. I look forward to seeing it in action.
> 
> 
>>On Tuesday, 22 January 2019, 12:05:24 GMT, Thiemo Kreuz <
>> thiemo.kr...@wikimedia.de> wrote:
>> 
>>> Fundamentally broken sounds like a bit of a stretch.
>> 
>> A process that annoys people based on nothing but the fact that they
>> happened to be the last one touching a file *is* fundamentally broken.
>> This is not how anyone should look for reviewers, neither manually nor
>> automatically.
>> 
>> Here is a thought experiment: We could send review requests to the
>> *least* active users that are still around, but *never* touched a
>> file. The positive effects of such an approach include:
>> * More people get familiar with the code.
>> * Knowledge gets spread more evenly.
>> * Bottlenecks and bus factors get reduced.
>> * These people probably have more time.
>> * Review requests are spread more evenly.
>> * Workload is spread more evenly.
>> 
>> Still sounds like a bad idea? Sure, because it is. Now tell me: How is
>> it more clever to do the *opposite* and dump review requests on people
>> that have to much workload already?
>> 
>> At this point I don't care any more if we are talking about a fully
>> automated process or a suggest button. Both are targeting the wrong
>> people.
>> 
>>> it was probably working quite well for our less-trafficked repositories.
>> 
>> What is the difference between being the last one fixing a typo in a
>> low-traffic vs. high-traffic repository? In both cases it's the wrong
>> person.
>> 
>> Thiemo
>> 
>> ___
>> 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
> 
> 
> 
> -- 
> Niharika
> Product Manager
> Community Tech
> 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

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Niharika Kohli]

On Tue, Jan 22, 2019 at 6:12 PM Paladox via Wikitech-l <
wikitech-l@lists.wikimedia.org> wrote:

>  What your saying is making me think I’m wasting my time on improving this
> extension.
> Also other users that have spoken to me have thought this extension is
> great but could do with improvements which I am doing. We need to think of
> new users and how to improve there experence. The task was opened for a
> long while yet no one commented on it.
> I agree with legoktm feedback.
> “A process that annoys people based on nothing but the fact that
> theyhappened to be the last one touching a file *is* fundamentally broken.”
> yes hence why I’ve been making improvements by adding a button which is
> better then nothing right?
> As chad mentions it has no idea what is a typo fix compared to other
> things as it’s not A.I.
>

Thanks for working on this, Paladox. I think this can be a really useful
feature for newcomers and experienced developers alike, if implemented
well. I look forward to seeing it in action.


> On Tuesday, 22 January 2019, 12:05:24 GMT, Thiemo Kreuz <
> thiemo.kr...@wikimedia.de> wrote:
>
>  > Fundamentally broken sounds like a bit of a stretch.
>
> A process that annoys people based on nothing but the fact that they
> happened to be the last one touching a file *is* fundamentally broken.
> This is not how anyone should look for reviewers, neither manually nor
> automatically.
>
> Here is a thought experiment: We could send review requests to the
> *least* active users that are still around, but *never* touched a
> file. The positive effects of such an approach include:
> * More people get familiar with the code.
> * Knowledge gets spread more evenly.
> * Bottlenecks and bus factors get reduced.
> * These people probably have more time.
> * Review requests are spread more evenly.
> * Workload is spread more evenly.
>
> Still sounds like a bad idea? Sure, because it is. Now tell me: How is
> it more clever to do the *opposite* and dump review requests on people
> that have to much workload already?
>
> At this point I don't care any more if we are talking about a fully
> automated process or a suggest button. Both are targeting the wrong
> people.
>
> > it was probably working quite well for our less-trafficked repositories.
>
> What is the difference between being the last one fixing a typo in a
> low-traffic vs. high-traffic repository? In both cases it's the wrong
> person.
>
> Thiemo
>
> ___
> 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



-- 
Niharika
Product Manager
Community Tech
Wikimedia Foundation
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Thiemo Kreuz]

> […] "people who have worked on this code before" is an excellent metric by 
> which to find people to review your code.

Sure. But this is neither what I wrote, nor what the plugin does, nor
what can be done programmatically in the first place, as you
conveniently pointed out yourself:

> […] we can't possibly know what was a one-line typofix and what was a one 
> line that saved us 50% of execution time on all pages. At least not 
> programmatically.

Kind regards
Thiemo

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Physikerwelt]

I suggest discussing the implementation details on phabricator.
Moreover, I second Lucas point on the tone.
physikerwelt
https://www.mediawiki.org/wiki/Code_of_Conduct

physikerwelt
https://www.mediawiki.org/wiki/Code_of_Conduct

On Tue, Jan 22, 2019 at 1:43 PM Lucas Werkmeister
 wrote:
>
> Am Di., 22. Jan. 2019 um 13:25 Uhr schrieb Chad :
>
> > Dumb straw man.
> >
>
> can we avoid this tone? thanks
>
> Who said these people have too much workload?
>
>
> Um, Thiemo himself has said this? Are you going to tell him that he’s wrong
> about his own workload?
>
> The blame attribution has zero insight into how
> > busy someone is.
> >
>
> Correct, which is why it’s a bad idea to let it run loose and add people
> who are already busy enough as reviewers.
>
>
> > If it's a low-traffic repository there's likely to be fewer overall
> > contributors.
> > Fewer contributors increases the likelihood of people being qualified to
> > review--whereas a high-traffic repo is more likely to have drive-by
> > contributor less capable.
> >
>
> Well, many drive-by contributions are tree-wide: they are applied to a
> large set of repositories collectively, e. g. all Wikimedia-deployed
> extensions or even all repositories. If a repository has generally low
> traffic, then these tree-wide drive-by contributions will make up a larger
> ratio of its commits than in repositories with more repository-specific
> commits.
>
> I’m not sure if I phrased this well, but if repository A has 1000 specific
> commits and 10 drive-by commits, and repository B has 20 specific commits
> and the same 10 drive-by commits, then the drive-by commits will be ⅓ of
> all commits in repository B but less than .1% in repository A.
>
>
> > And if it's just one-line typofixing it'd be ideal to exclude those from
> > the
> > blame list--but we can't possibly know what was a one-line typofix and
> > what was a one line that saved us 50% of execution time on all pages.
> > At least not programmatically.
> >
>
> True to some extent, but then we should err on the side of not adding the
> reviewer, no? Otherwise we run the risk of overwhelming them with changes
> they’re not even qualified to review, even if they had the time.
>
>
> > Honestly, if you think "people who've edited the code in the past" are a
> > poor
> > person to ask for review then you do not understand how code review works.
> >
>
> Suggesting that Thiemo doesn’t understand how code review works is… bold,
> in my opinion, let’s put it that way. May I point out that he’s one of the
> top +2ers across all MediaWiki extensions
> ?
>
> Cheers,
> Lucas
>
>
> --
> Lucas Werkmeister
> Full Stack Developer
>
> Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin
> Phone: +49 (0)30 219 158 26-0
> https://wikimedia.de
>
> Imagine a world in which every single human being can freely share in the
> sum of all knowledge. Help us to achieve our vision!
> https://spenden.wikimedia.de
>
> Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V.
> Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter
> der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für
> Körperschaften I Berlin, Steuernummer 27/029/42207.
> ___
> 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] Gerrit now automatically adds reviewers

[Lucas Werkmeister]

Am Di., 22. Jan. 2019 um 13:25 Uhr schrieb Chad :

> Dumb straw man.
>

can we avoid this tone? thanks

Who said these people have too much workload?


Um, Thiemo himself has said this? Are you going to tell him that he’s wrong
about his own workload?

The blame attribution has zero insight into how
> busy someone is.
>

Correct, which is why it’s a bad idea to let it run loose and add people
who are already busy enough as reviewers.


> If it's a low-traffic repository there's likely to be fewer overall
> contributors.
> Fewer contributors increases the likelihood of people being qualified to
> review--whereas a high-traffic repo is more likely to have drive-by
> contributor less capable.
>

Well, many drive-by contributions are tree-wide: they are applied to a
large set of repositories collectively, e. g. all Wikimedia-deployed
extensions or even all repositories. If a repository has generally low
traffic, then these tree-wide drive-by contributions will make up a larger
ratio of its commits than in repositories with more repository-specific
commits.

I’m not sure if I phrased this well, but if repository A has 1000 specific
commits and 10 drive-by commits, and repository B has 20 specific commits
and the same 10 drive-by commits, then the drive-by commits will be ⅓ of
all commits in repository B but less than .1% in repository A.


> And if it's just one-line typofixing it'd be ideal to exclude those from
> the
> blame list--but we can't possibly know what was a one-line typofix and
> what was a one line that saved us 50% of execution time on all pages.
> At least not programmatically.
>

True to some extent, but then we should err on the side of not adding the
reviewer, no? Otherwise we run the risk of overwhelming them with changes
they’re not even qualified to review, even if they had the time.


> Honestly, if you think "people who've edited the code in the past" are a
> poor
> person to ask for review then you do not understand how code review works.
>

Suggesting that Thiemo doesn’t understand how code review works is… bold,
in my opinion, let’s put it that way. May I point out that he’s one of the
top +2ers across all MediaWiki extensions
?

Cheers,
Lucas


-- 
Lucas Werkmeister
Full Stack Developer

Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin
Phone: +49 (0)30 219 158 26-0
https://wikimedia.de

Imagine a world in which every single human being can freely share in the
sum of all knowledge. Help us to achieve our vision!
https://spenden.wikimedia.de

Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V.
Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter
der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für
Körperschaften I Berlin, Steuernummer 27/029/42207.
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 What your saying is making me think I’m wasting my time on improving this 
extension.
Also other users that have spoken to me have thought this extension is great 
but could do with improvements which I am doing. We need to think of new users 
and how to improve there experence. The task was opened for a long while yet no 
one commented on it.
I agree with legoktm feedback.
“A process that annoys people based on nothing but the fact that theyhappened 
to be the last one touching a file *is* fundamentally broken.”
yes hence why I’ve been making improvements by adding a button which is better 
then nothing right?
As chad mentions it has no idea what is a typo fix compared to other things as 
it’s not A.I.

On Tuesday, 22 January 2019, 12:05:24 GMT, Thiemo Kreuz 
 wrote:  
 
 > Fundamentally broken sounds like a bit of a stretch.

A process that annoys people based on nothing but the fact that they
happened to be the last one touching a file *is* fundamentally broken.
This is not how anyone should look for reviewers, neither manually nor
automatically.

Here is a thought experiment: We could send review requests to the
*least* active users that are still around, but *never* touched a
file. The positive effects of such an approach include:
* More people get familiar with the code.
* Knowledge gets spread more evenly.
* Bottlenecks and bus factors get reduced.
* These people probably have more time.
* Review requests are spread more evenly.
* Workload is spread more evenly.

Still sounds like a bad idea? Sure, because it is. Now tell me: How is
it more clever to do the *opposite* and dump review requests on people
that have to much workload already?

At this point I don't care any more if we are talking about a fully
automated process or a suggest button. Both are targeting the wrong
people.

> it was probably working quite well for our less-trafficked repositories.

What is the difference between being the last one fixing a typo in a
low-traffic vs. high-traffic repository? In both cases it's the wrong
person.

Thiemo

___
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] Gerrit now automatically adds reviewers

[Chad]

On Tue, Jan 22, 2019 at 4:05 AM Thiemo Kreuz 
wrote:

> > Fundamentally broken sounds like a bit of a stretch.
>
> A process that annoys people based on nothing but the fact that they
> happened to be the last one touching a file *is* fundamentally broken.
> This is not how anyone should look for reviewers, neither manually nor
> automatically.


It isn't? I figured "people who have worked on this code before" is an
excellent metric by which to find people to review your code. It's certainly
who I'd look to help me if I didn't just _know_ who to ask.


>
>
Here is a thought experiment: We could send review requests to the
> *least* active users that are still around, but *never* touched a
> file. The positive effects of such an approach include:
> * More people get familiar with the code.
> * Knowledge gets spread more evenly.
> * Bottlenecks and bus factors get reduced.
> * These people probably have more time.
> * Review requests are spread more evenly.
> * Workload is spread more evenly.
>
> Still sounds like a bad idea? Sure, because it is.


Dumb straw man.


> Now tell me: How is
> it more clever to do the *opposite* and dump review requests on people
> that have to much workload already?
>

Who said these people have too much workload? Just because they've
made a commit before? The blame attribution has zero insight into how
busy someone is.


> At this point I don't care any more if we are talking about a fully
> automated process or a suggest button. Both are targeting the wrong
> people.


>
> it was probably working quite well for our less-trafficked repositories.
>
> What is the difference between being the last one fixing a typo in a
> low-traffic vs. high-traffic repository? In both cases it's the wrong
> person.
>

If it's a low-traffic repository there's likely to be fewer overall
contributors.
Fewer contributors increases the likelihood of people being qualified to
review--whereas a high-traffic repo is more likely to have drive-by
contributor less capable.

And if it's just one-line typofixing it'd be ideal to exclude those from the
blame list--but we can't possibly know what was a one-line typofix and
what was a one line that saved us 50% of execution time on all pages.
At least not programmatically.

Honestly, if you think "people who've edited the code in the past" are a
poor
person to ask for review then you do not understand how code review works.

-Chad
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Thiemo Kreuz]

> Fundamentally broken sounds like a bit of a stretch.

A process that annoys people based on nothing but the fact that they
happened to be the last one touching a file *is* fundamentally broken.
This is not how anyone should look for reviewers, neither manually nor
automatically.

Here is a thought experiment: We could send review requests to the
*least* active users that are still around, but *never* touched a
file. The positive effects of such an approach include:
* More people get familiar with the code.
* Knowledge gets spread more evenly.
* Bottlenecks and bus factors get reduced.
* These people probably have more time.
* Review requests are spread more evenly.
* Workload is spread more evenly.

Still sounds like a bad idea? Sure, because it is. Now tell me: How is
it more clever to do the *opposite* and dump review requests on people
that have to much workload already?

At this point I don't care any more if we are talking about a fully
automated process or a suggest button. Both are targeting the wrong
people.

> it was probably working quite well for our less-trafficked repositories.

What is the difference between being the last one fixing a typo in a
low-traffic vs. high-traffic repository? In both cases it's the wrong
person.

Thiemo

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Kunal Mehta]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

Hi,

On 1/18/19 6:11 AM, Tyler Cipriani wrote:
> In the interim, project-owners are able to opt-in to using the 
> reviewers-by-blame plugin on a per-project basis on their project
> admin page in Gerrit.

I'm happy to volunteer any of the projects I help maintain as a guinea
pig once modifications to the plugin are ready for wider testing.

> Also, the Git Reviewer Bot[1] provides folks an opt-in method of 
> volunteering to review a subset of files in a particular repo.

IMO, the main place where the Reviewer-Bot falls short is that it
relies on a hardcoded list of file paths that often change and then
requires manual intervention. Using blame is nice in that it's automatic
.

> Getting code review as a new contributor is hard. Thanks for
> bearing with us.

I've spoken to plenty people that complain that their patch was never
merged, and after looking, it had no reviewers assigned :-(

My suggestions for the plugin:
* Require some manual interaction by the patch author, similar to
GitHub (I believe Paladox is already working on this)
* Some account-level opt-out for bots, people who have left the
movement, etc.
* Don't suggest people that have already been manually removed from
the patch
* Only suggest people that have been active on Gerrit (or maybe limit
it to that repo) in the past X months/years.
** For repositories that have no active development but a maintainer
who still checks email, using a time-based check might not work. In a
past experiment I looked at the last 100 merged patches.
* If feasible, introduce some kind of marking (hashtag? topic?) that
can be used in mass cleanup style commits so that they are ignored by
the blame so drive by contributors aren't suggested as reviewers. I
currently have a mail filter with a few different Gerrit topics (e.g.
bump-dev-deps) to separate those review requests from other ones.

Thanks to those that are working on this.
- -- Legoktm
-BEGIN PGP SIGNATURE-

iQIzBAEBCgAdFiEE2MtZ8F27ngU4xIGd8QX4EBsFJpsFAlxG2x8ACgkQ8QX4EBsF
JptONA//S4Ea2Fxsmm7ZrWs/yb+qSuYQQuAUnTnSUdCY2+rCcIsnhLvHaF/eGdFE
JEsjiq4bHpb58FG+oDvB3TAJxhrhs3Vi+6reZVONzKNS/cki//FGGiBjLqmd/5a6
8LVwswOUsJgiMig45ZL48nrpB9GorxJIj8PxhvzE7IuBWTdwUaiW1Wt0n2/6CNgj
Rj9mMv+cPVhy19itzQ5pwwxmeA/wRzqvnPBUFgGdigrXivtQhzw5tFg/KXiO/kLc
90Wb2ubZpHNvW4EN+TYDMacy9Y+kuluvkynK8fc40xQHf4eS98W3bTnihO6NFL+j
c2LqEVLrGxOHa70xt6K2poJ2JbgCYGlyzYLbxU9ug0gWh4HT+Fwo/keNLi41WyJC
UibnObplCBIna0aZ2xsgoo6F1rS5UUb/3TAyTR8Lx8ceeoteWNtiR/rdFOLR0Br8
vUyWkFJ9sESW75UdeiU3CpHoBisK9lKRa+zdhjAqwwUKoOZDdjq7iKfggK2Yl5Hp
1iF8ARDmtiTN1XgaN64vT7VXpv5+RrWXHFki9WCapWhSP3PCovBp0fTB1MMhAZq3
N3kACRPJR1NrrVw/ZUdCm85cDbFdueNp7Pl2eV5C5Bqj4nyaL89t0L2Uinz3snYI
Y3lfFRB8I74CamwyTAzTQAILMMXVj7uqCvR0j6lTix+czmMKpao=
=QEZv
-END PGP SIGNATURE-

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Gergo Tisza]

This discussion could do with less theatrics IMO.
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 No, I added in a config so that it would disable automatically adding 
reviewers instead relying on users to press “Suggest Reviewers” button.
On Tuesday, 22 January 2019, 08:11:23 GMT, Thiemo Kreuz 
 wrote:  
 
 > […] im adding that functionality to reviewers-by-blame.

I'm afraid I don't understand. Does this mean all the issues that make
the plugin send passive-aggressive, misattributed spam will still be
in place, possibly hitting peoples inboxes again any time somebody
decides it would be a good idea to turn this feature set on again? My
impression was more that all the ideas that have been shared in this
email thread describe an *entirely* different approach, following
entirely different ideas, with an entirely different UI, and entirely
different setup. What's the point of stuffing this into the same,
fundamentally broken codebase?

Sysops, please, for our sanity, do not let anybody enable this toxic
plugin again.

Best
Thiemo

___
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] Gerrit now automatically adds reviewers

[Chad]

Fundamentally broken sounds like a bit of a stretch.

In fact, it was probably working quite well for our less-trafficked
repositories. But the issues identified must be fixed and decent file
exclusion rules written before it goes back on for the active ones.

-Chad

On Jan 22, 2019 12:11 AM, "Thiemo Kreuz"  wrote:

> […] im adding that functionality to reviewers-by-blame.

I'm afraid I don't understand. Does this mean all the issues that make
the plugin send passive-aggressive, misattributed spam will still be
in place, possibly hitting peoples inboxes again any time somebody
decides it would be a good idea to turn this feature set on again? My
impression was more that all the ideas that have been shared in this
email thread describe an *entirely* different approach, following
entirely different ideas, with an entirely different UI, and entirely
different setup. What's the point of stuffing this into the same,
fundamentally broken codebase?

Sysops, please, for our sanity, do not let anybody enable this toxic
plugin again.


Best
Thiemo

___
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] Gerrit now automatically adds reviewers

[Thiemo Kreuz]

> […] im adding that functionality to reviewers-by-blame.

I'm afraid I don't understand. Does this mean all the issues that make
the plugin send passive-aggressive, misattributed spam will still be
in place, possibly hitting peoples inboxes again any time somebody
decides it would be a good idea to turn this feature set on again? My
impression was more that all the ideas that have been shared in this
email thread describe an *entirely* different approach, following
entirely different ideas, with an entirely different UI, and entirely
different setup. What's the point of stuffing this into the same,
fundamentally broken codebase?

Sysops, please, for our sanity, do not let anybody enable this toxic
plugin again.

Best
Thiemo

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 Change is now available at: 
https://gerrit-review.googlesource.com/c/plugins/reviewers-by-blame/+/210812 
and  
https://gerrit-review.googlesource.com/c/plugins/reviewers-by-blame/+/211052
On Monday, 21 January 2019, 19:46:41 GMT, Paladox via Wikitech-l 
 wrote:  
 
  Nope, im adding that functionality to reviewers-by-blame.
Unless y'all want a plugin where you can define reviewers each repo and a user 
can press "Suggest Reviewers", but it would probably be best to use 
Reviewers-By-Blame once all the feedback has been addressed.
    On Monday, 21 January 2019, 19:37:24 GMT, Thiemo Kreuz 
 wrote:  
 
 > […] i have a working prototype working ("Suggest Reviewer") button.

Ok, but that's another extension then. Can we kill the bad one, please?

Best
Thiemo

___
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] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 Nope, im adding that functionality to reviewers-by-blame.
Unless y'all want a plugin where you can define reviewers each repo and a user 
can press "Suggest Reviewers", but it would probably be best to use 
Reviewers-By-Blame once all the feedback has been addressed.
On Monday, 21 January 2019, 19:37:24 GMT, Thiemo Kreuz 
 wrote:  
 
 > […] i have a working prototype working ("Suggest Reviewer") button.

Ok, but that's another extension then. Can we kill the bad one, please?

Best
Thiemo

___
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] Gerrit now automatically adds reviewers

[Thiemo Kreuz]

> […] i have a working prototype working ("Suggest Reviewer") button.

Ok, but that's another extension then. Can we kill the bad one, please?

Best
Thiemo

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 FYI i have a working prototype working ("Suggest Reviewer") button.
On Monday, 21 January 2019, 16:32:35 GMT, Paladox via Wikitech-l 
 wrote:  
 
  I’m currently working on addressing all the feedback as fast as I can.
I honestly think this extension is great especially for new users, who do not 
know they need reviewers or who would review there change. Granted this 
extension has some problems hence why feature requests were filed against the 
extension.
    On Monday, 21 January 2019, 15:41:53 GMT, MA  wrote:  
 
 Hello,

El lun., 21 ene. 2019 a las 10:14, Thiemo Kreuz
() escribió:
>
> Can I please ask again to *ban* this bad plugin entirely from our
> systems? Having it sit there for anybody to enable again is a ticking
> time bomb. It will start sending out the same misattributed,
> uncontrollable, aggressive fake request spam again. I don't want
> anybody to experience something like this again.
>
> Look for a plugin that makes suggestions, please.
>
> Best
> Thiemo

I think Thiemo makes a good point again. As things stand now support
disabling/removing the plugin entirely from our gerrit install.

Thank you, M.

___
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] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 I’m currently working on addressing all the feedback as fast as I can.
I honestly think this extension is great especially for new users, who do not 
know they need reviewers or who would review there change. Granted this 
extension has some problems hence why feature requests were filed against the 
extension.
On Monday, 21 January 2019, 15:41:53 GMT, MA  wrote:  
 
 Hello,

El lun., 21 ene. 2019 a las 10:14, Thiemo Kreuz
() escribió:
>
> Can I please ask again to *ban* this bad plugin entirely from our
> systems? Having it sit there for anybody to enable again is a ticking
> time bomb. It will start sending out the same misattributed,
> uncontrollable, aggressive fake request spam again. I don't want
> anybody to experience something like this again.
>
> Look for a plugin that makes suggestions, please.
>
> Best
> Thiemo

I think Thiemo makes a good point again. As things stand now support
disabling/removing the plugin entirely from our gerrit install.

Thank you, M.

___
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] Gerrit now automatically adds reviewers

[MA]

Hello,

El lun., 21 ene. 2019 a las 10:14, Thiemo Kreuz
() escribió:
>
> Can I please ask again to *ban* this bad plugin entirely from our
> systems? Having it sit there for anybody to enable again is a ticking
> time bomb. It will start sending out the same misattributed,
> uncontrollable, aggressive fake request spam again. I don't want
> anybody to experience something like this again.
>
> Look for a plugin that makes suggestions, please.
>
> Best
> Thiemo

I think Thiemo makes a good point again. As things stand now support
disabling/removing the plugin entirely from our gerrit install.

Thank you, M.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Thiemo Kreuz]

Can I please ask again to *ban* this bad plugin entirely from our
systems? Having it sit there for anybody to enable again is a ticking
time bomb. It will start sending out the same misattributed,
uncontrollable, aggressive fake request spam again. I don't want
anybody to experience something like this again.

Look for a plugin that makes suggestions, please.

Best
Thiemo

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Merlijn van Deen (valhallasw)]

On Fri, 18 Jan 2019 at 07:58, Giuseppe Lavagetto 
wrote:

> The amount of noise will prevent me from being able to notice anyone's
> review request. I think it's going to be the same for other developers - I
> don't want to imagine what the inbox of a long-time mediawiki-core
> contributor must look like!
>
> What I fear is that the flood of reviews will make everyone just dull to
> notifications, obtaining the exact opposite effect that was intended. I say
> this because  I auto added myself to all reviews in operations/puppet[1] in
> the past, which resulted in me ignoring all code review requests.
>
>
Reading this thread -- and Guiseppes comment above in particular, made me
think a bit on how we do code review, and how automatically adding
reviewers may be counterproductive. This includes the Gerrit Reviewer Bot:
even though it is opt-in, it may still overwhelm the inbox of whoever opted
in, and if that person does not unsubscribe (but rather creates a filter to
move all the requests into a 'I don't have time to look at this now, but I
will review these later' mailbox - I have done this myself at some point,
which meant that in practice, I didn't review anything for that project
anymore), we end up in a situation where code is not actually reviewed in a
more timely manner.

One thing I wondered about is how big the problem of patches without
relevant reviewers is. This is not entirely trivial to query, but the best
I could come up with was looking for patches without any reviewer at all:
https://gerrit.wikimedia.org/r/q/status:open+-reviewerin:%2522Registered+Users%2522
. This is only a very small number of patches -- while the number of
patches that have V+1 CR=0 Age > 1 month is much larger:
https://gerrit.wikimedia.org/r/q/status:open+label:Verified%253E%253D0+label:Code-Review%253D0+age:1month

This suggests to me that the problem is not a lack of reviewers added, but
a lack of reviewing :-). This might be due to the wrong reviewers being
added (which an improved version of the auto-reviewer plugin could solve),
but it might also just be that the reviewers don't have enough time to
actually perform the reviews. That includes myself -- I find it very
difficult to get started doing reviews on code I haven't looked at a while.
After all, it's much more fun to write code than to review it ;-)

How to solve that? I don't know -- I think initiatives such as Andre's
'Patchsets by new Gerrit contributors' emails help (as they focus on a
small number of changes). The same is true for a form of gamification (such
as the statistics in the previous Thank You day threads).

In general, I believe that trying something new (which includes trying the
Gerrit plugin!) is going to be beneficial, as otherwise we never discover
which directions improve things (and which ones don't). After all, the
Reviewer Bot is already 8 years old, and it's unlikely that the thing I
hacked together back then is still the best solution now :-)

Merlijn
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Dan Garry (Deskana)]

Something new was tried in the hopes it'd be good, it turned out not to be
good, it was reverted, and now there's some discussion about how to make it
better. That's a successful process, not an unsuccessful one.

Given that this exact method of doing things is not only well-established
on the English Wikipedia but is also a recommended pattern (
bold-revert-discuss ), I'm not
sure why you think this would be unacceptable there.

Dan

On Fri, 18 Jan 2019 at 22:13, Pine W  wrote:

> I'm glad that this problematic change to communications was reverted.
>
> I would like to suggest that this is the type of change that, when being
> planned, should get a design review from a third party before coding
> starts, should go through at least one RFC before coding starts, and be
> widely communicated before coding starts and again a week or two before
> deployment. Involving TechCom might also be appropriate. It appears that
> none of those happened here. In terms of process this situation looks to me
> like it's inexcusable.
>
> In the English Wikipedia community, doing something like this would have a
> reasonable likelihood of costing an administrator their tools, and I hope
> that a similar degree of accountability is enforced in the engineering
> community. In particular, I expect engineering supervisors to follow
> established technical processes for changes that impact others' workflows,
> and if they decide to skip those processes without a compelling reason
> (such as a site stability problem) then I hope that they will be held
> accountable. Again, from my perspective, the failure to follow process here
> is inexcusable.
>
> Pine
> ( https://meta.wikimedia.org/wiki/User:Pine )
> ___
> 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] Gerrit now automatically adds reviewers

[Tyler Cipriani]

On 19-01-18 22:12:22, Pine W wrote:

I'm glad that this problematic change to communications was reverted.


Clarification: Enabling this plugin wasn't reverted, a configuration 
change was made to the default settings of the plugin.


Thanks to the helpful suggestions on this thread, it's my hope that the 
upstream plugin (in future) may contain additional configuration options 
to improve the usability of this plugin for everyone, including 
Wikimedia technical contributors.



I would like to suggest that this is the type of change that, when being
planned, should get a design review from a third party before coding
starts, should go through at least one RFC before coding starts, and be
widely communicated before coding starts and again a week or two before
deployment. Involving TechCom might also be appropriate. It appears that
none of those happened here. In terms of process this situation looks to me
like it's inexcusable.


As Chad mentioned this is a plugin developed by upstream Gerrit.

Enabling this plugin was tracked in Wikimedia's public Phabricator[0].

As is now well understood in hindsight, the default configuration of 
this plugin (as designed by Gerrit upstream) is far from optimal 
for Wikimedia technical contributors.


[0]. 


In the English Wikipedia community, doing something like this would have a
reasonable likelihood of costing an administrator their tools, and I hope
that a similar degree of accountability is enforced in the engineering
community. In particular, I expect engineering supervisors to follow
established technical processes for changes that impact others' workflows,
and if they decide to skip those processes without a compelling reason
(such as a site stability problem) then I hope that they will be held
accountable. Again, from my perspective, the failure to follow process here
is inexcusable.


As was pointed out by others: it's difficult to make a comparison 
between the English Wikipedia community and the Wikimedia technical 
contributors community (although many folks belong to both groups). I 
don't believe holding individuals to a post hoc set of standards creates 
a healthy community in any case.


I do agree that technical contributors should be accountable. That is, 
technical contributors should strive to be responsive to issues when 
they arise (as issues will arise when attempting to accomplish goals in 
a technical space).


-- Tyler


signature.asc
Description: PGP signature
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Daniel Kinzler]

Am 19.01.19 um 03:46 schrieb Chad:
> All examples of files that are edited by dozens of folks. Folks who could
> very well be unqualified to review your change and/or completely
> uninterested in it at all.

Coming to think of it, it would be much more useful to base the suggestion on
who has *reviewed* and *merged* changes to the file before, as opposed to who
has *authored* patches.

People who have merged before may merge again. People who have authored small
changes to lots of files don't want to be spammed with review requests.


-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
Wikimedia Foundation

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Gergo Tisza]

On Fri, Jan 18, 2019 at 6:46 PM Chad  wrote:

> There _must_ be a way to disable this for certain files. Great examples:
>
> site.pp in puppet
> en.json language file in MW core
> CommonSettings or InitialiseSettings in wmf-config
>

There *is* a way to disable it. See plugins docs:
https://gerrit.wikimedia.org/r/plugins/reviewers-by-blame/Documentation/config.md

IMO the two blockers are having the plugin act under its own username and
providing some form of user optout. The other issues are annoying but can
be worked around (or opted out from in the worst case).
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Chad]

On Fri, Jan 18, 2019 at 2:13 PM Pine W  wrote:

> I would like to suggest that this is the type of change that, when being
> planned, should get a design review from a third party before coding
> starts, should go through at least one RFC before coding starts, and be
> widely communicated before coding starts and again a week or two before
> deployment.


There was no coding to start--it's an upstream plugin.

One that I considered enabling ages ago, fwiw. I didn't because of the very
issues outlined in this thread.

There _must_ be a way to disable this for certain files. Great examples:

site.pp in puppet
en.json language file in MW core
CommonSettings or InitialiseSettings in wmf-config

All examples of files that are edited by dozens of folks. Folks who could
very well be unqualified to review your change and/or completely
uninterested in it at all.

I did enable the reviewers (not by blame) plugin ages ago. This allows
people to opt in with much more granularity (and would remove the need for
the bot)

-Chad
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Stas Malyshev]

Hi!

> On Fri, Jan 18, 2019 at 10:13 PM Pine W  wrote:
> 
>> I'm glad that this problematic change to communications was reverted.
>>
>> I would like to suggest that this is the type of change that, when being
>> planned, should get a design review from a third party before coding
>> starts, should go through at least one RFC before coding starts, and be

I think there's no reason to make it bigger deal than it is. There was a
feature that people thought would be good, turned out it is not as good
as expected, so it was disabled. Nothing broken, nobody hurt (well,
maybe except some inboxes...). I don't think there's a reason to
describe this situation as "inexcusable". Sometimes something that we
expect to work one way and be useful turns out different way and things
that seemed to be excellent idea turn out to be very bad one. And we
have to adjust, and this is normal. We don't like such situations, but
we know they happen, and as long as they are handled properly - and I
think in this case it was - there's no reason to make it more than it is.

>> reasonable likelihood of costing an administrator their tools, and I hope
>> that a similar degree of accountability is enforced in the engineering

I hope not. Expecting unreasonable perfection from people and processes
and overreacting when inevitable problems happen will only lead to
frustration, failure and stagnation. Every failure has some lesson to
learn, but the lesson shouldn't be "let's find somebody to punish". I am
not sure if that was the intent, but it kinda felt this way to me. And I
don't think this is warranted neither in general nor in particular case.

Thanks,
-- 
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] [Engineering] Gerrit now automatically adds reviewers

[bawolff]

Umm, No.

--
Bawolff

On Fri, Jan 18, 2019 at 10:13 PM Pine W  wrote:

> I'm glad that this problematic change to communications was reverted.
>
> I would like to suggest that this is the type of change that, when being
> planned, should get a design review from a third party before coding
> starts, should go through at least one RFC before coding starts, and be
> widely communicated before coding starts and again a week or two before
> deployment. Involving TechCom might also be appropriate. It appears that
> none of those happened here. In terms of process this situation looks to me
> like it's inexcusable.
>
> In the English Wikipedia community, doing something like this would have a
> reasonable likelihood of costing an administrator their tools, and I hope
> that a similar degree of accountability is enforced in the engineering
> community. In particular, I expect engineering supervisors to follow
> established technical processes for changes that impact others' workflows,
> and if they decide to skip those processes without a compelling reason
> (such as a site stability problem) then I hope that they will be held
> accountable. Again, from my perspective, the failure to follow process here
> is inexcusable.
>
> Pine
> ( https://meta.wikimedia.org/wiki/User:Pine )
> ___
> 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] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 "should get a design review from a third party before coding starts" what do 
you mean by that?
Also may i remind you that gerrit is not a encyclopedia, it's a code review 
system.
So what ever process is on the encyclopedia is not the same here.
Also please can we leave the threats out. It wasn't deployed to annoy users, it 
was deployed to help. Obviously we didn't know it was going to turn out like 
this, hence why it was made opt in for all projects today.


On Friday, 18 January 2019, 22:13:38 GMT, Pine W  
wrote:  
 
 I'm glad that this problematic change to communications was reverted.

I would like to suggest that this is the type of change that, when being
planned, should get a design review from a third party before coding
starts, should go through at least one RFC before coding starts, and be
widely communicated before coding starts and again a week or two before
deployment. Involving TechCom might also be appropriate. It appears that
none of those happened here. In terms of process this situation looks to me
like it's inexcusable.

In the English Wikipedia community, doing something like this would have a
reasonable likelihood of costing an administrator their tools, and I hope
that a similar degree of accountability is enforced in the engineering
community. In particular, I expect engineering supervisors to follow
established technical processes for changes that impact others' workflows,
and if they decide to skip those processes without a compelling reason
(such as a site stability problem) then I hope that they will be held
accountable. Again, from my perspective, the failure to follow process here
is inexcusable.

Pine
( https://meta.wikimedia.org/wiki/User:Pine )
___
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] Gerrit now automatically adds reviewers

[Pine W]

I'm glad that this problematic change to communications was reverted.

I would like to suggest that this is the type of change that, when being
planned, should get a design review from a third party before coding
starts, should go through at least one RFC before coding starts, and be
widely communicated before coding starts and again a week or two before
deployment. Involving TechCom might also be appropriate. It appears that
none of those happened here. In terms of process this situation looks to me
like it's inexcusable.

In the English Wikipedia community, doing something like this would have a
reasonable likelihood of costing an administrator their tools, and I hope
that a similar degree of accountability is enforced in the engineering
community. In particular, I expect engineering supervisors to follow
established technical processes for changes that impact others' workflows,
and if they decide to skip those processes without a compelling reason
(such as a site stability problem) then I hope that they will be held
accountable. Again, from my perspective, the failure to follow process here
is inexcusable.

Pine
( https://meta.wikimedia.org/wiki/User:Pine )
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Thiemo Kreuz]

> Gerrit no longer automatically adds reviewers […]

Thank you very much for reacting so fast.

> https://gerrit.wikimedia.org/r/485184

I'm not sure if the list of issues in this commit message is meant to
be complete. But I noticed it misses the bug I found the most
annoying: The plugin doesn't get activated once per patch, but for
every patch set, and ignores every human intervention that might have
been done to this point, most notably when reviewers have manually
been removed. There are many ways to fix this, but the current
behavior is unacceptable.

It is due to this bug that I wish we would revoke this bad plugin
entirely. I don't think anybody should be able to run it as this would
open the exact same issues again, just on a smaller scale, but still
hurting the same people.

Best
Thiemo

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 I've forwarded jynus feedback upstream at 
https://bugs.chromium.org/p/gerrit/issues/detail?id=10337

On Friday, 18 January 2019, 15:01:09 GMT, Paladox via Wikitech-l 
 wrote:  
 
  The patch i linked in my other email does what you are all suggesting :) (opt 
out per project).
If done through All-Projects it will opt you out of all projects.
    On Friday, 18 January 2019, 14:44:50 GMT, Daniel Kinzler 
 wrote:  
 
 Am 18.01.19 um 15:25 schrieb Jaime Crespo:
> Let me suggest one workflow that may work with this feature- Adding a
> button, for example, with "Suggest reviewers" which you can press to get
> this effect. 

Yes.

It could also be doneautomatically when a patch did not get any attention for a
week or so.

In any case, there should be a way to opt out. Ideally, per project.


-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
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 mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 The patch i linked in my other email does what you are all suggesting :) (opt 
out per project).
If done through All-Projects it will opt you out of all projects.
On Friday, 18 January 2019, 14:44:50 GMT, Daniel Kinzler 
 wrote:  
 
 Am 18.01.19 um 15:25 schrieb Jaime Crespo:
> Let me suggest one workflow that may work with this feature- Adding a
> button, for example, with "Suggest reviewers" which you can press to get
> this effect. 

Yes.

It could also be doneautomatically when a patch did not get any attention for a
week or so.

In any case, there should be a way to opt out. Ideally, per project.


-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
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

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Daniel Kinzler]

Am 18.01.19 um 15:25 schrieb Jaime Crespo:
> Let me suggest one workflow that may work with this feature- Adding a
> button, for example, with "Suggest reviewers" which you can press to get
> this effect. 

Yes.

It could also be doneautomatically when a patch did not get any attention for a
week or so.

In any case, there should be a way to opt out. Ideally, per project.


-- 
Daniel Kinzler
Principal Software Engineer, Core Platform
Wikimedia Foundation

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Ariel Glenn WMF]

In the meantime, I would encourage those who have not looked at the Git
Reviewer Bot page in a while, to do so and to add any updates.

Ariel

On Fri, Jan 18, 2019 at 4:12 PM Tyler Cipriani 
wrote:

> Hi all,
>
> Gerrit no longer automatically adds reviewers[0]. Unfortunately, this
> plugin appears (given the replies on this thread) to be missing key
> features needed to be useful for us at this time. Apologies to those
> folks whose inboxes were destroyed.
>
> I would like to re-enable this plugin at some point, provided the
> features identified in this thread are added (perhaps also an
> "X-Gerrit-reviewers-by-blame: 1" email header, or subject line to make
> filtering these messages easier).
>
> In the interim, project-owners are able to opt-in to using the
> reviewers-by-blame plugin on a per-project basis on their project admin
> page in Gerrit.
>
> Also, the Git Reviewer Bot[1] provides folks an opt-in method of
> volunteering to review a subset of files in a particular repo.
>
> Getting code review as a new contributor is hard. Thanks for bearing
> with us.
>
> -- Tyler
>
> [0]. 
> [1]. 
>
> On 19-01-17 13:51:58, Greg Grossmeier wrote:
> >Hello,
> >
> >Yesterday we (the Release Engineering team) enabled a Gerrit plugin that
> >will automatically add reviewers to your changes based on who previously
> >has committed changes to the file.
> >
> >For more, please read the blog post at:
> >
> https://phabricator.wikimedia.org/phame/post/view/139/gerrit_now_automatically_adds_reviewers/
> >
> >NOTE: There are a couple requests from us open upstream to improve the
> >plugin[0], we'll incorporate those improvements when they are released.
> >
> >On behalf of the rest of the Release Engineering Team[1],
> >
> >Greg
> >
> >[0] https://phabricator.wikimedia.org/T101131#4890023
> >[1] As well as Paladox, a Wikimedia volunteer with strong ties to
> >upstream Gerrit.
> >
> >--
> >| Greg GrossmeierGPG: B2FA 27B1 F7EB D327 6B8E |
> >| Release Team ManagerA18D 1138 8E47 FAC8 1C7D |
> >
> >___
> >Engineering mailing list
> >engineer...@lists.wikimedia.org
> >https://lists.wikimedia.org/mailman/listinfo/engineering
> ___
> 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] [Engineering] Gerrit now automatically adds reviewers

[Jaime Crespo]

On Fri, Jan 18, 2019 at 3:12 PM Tyler Cipriani 
wrote:

> I would like to re-enable this plugin at some point, provided the
> features identified in this thread are added (perhaps also an
> "X-Gerrit-reviewers-by-blame: 1" email header, or subject line to make
> filtering these messages easier).


Let me suggest one workflow that may work with this feature- Adding a
button, for example, with "Suggest reviewers" which you can press to get
this effect. Or doing it automatically if your history only has less than X
CRs sent. What do you think?
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Tyler Cipriani]

Hi all,

Gerrit no longer automatically adds reviewers[0]. Unfortunately, this 
plugin appears (given the replies on this thread) to be missing key 
features needed to be useful for us at this time. Apologies to those 
folks whose inboxes were destroyed.


I would like to re-enable this plugin at some point, provided the 
features identified in this thread are added (perhaps also an 
"X-Gerrit-reviewers-by-blame: 1" email header, or subject line to make 
filtering these messages easier).


In the interim, project-owners are able to opt-in to using the 
reviewers-by-blame plugin on a per-project basis on their project admin 
page in Gerrit.


Also, the Git Reviewer Bot[1] provides folks an opt-in method of 
volunteering to review a subset of files in a particular repo.


Getting code review as a new contributor is hard. Thanks for bearing 
with us.


-- Tyler

[0]. 
[1]. 

On 19-01-17 13:51:58, Greg Grossmeier wrote:

Hello,

Yesterday we (the Release Engineering team) enabled a Gerrit plugin that
will automatically add reviewers to your changes based on who previously
has committed changes to the file.

For more, please read the blog post at:
https://phabricator.wikimedia.org/phame/post/view/139/gerrit_now_automatically_adds_reviewers/

NOTE: There are a couple requests from us open upstream to improve the
plugin[0], we'll incorporate those improvements when they are released.

On behalf of the rest of the Release Engineering Team[1],

Greg

[0] https://phabricator.wikimedia.org/T101131#4890023
[1] As well as Paladox, a Wikimedia volunteer with strong ties to
upstream Gerrit.

--
| Greg GrossmeierGPG: B2FA 27B1 F7EB D327 6B8E |
| Release Team ManagerA18D 1138 8E47 FAC8 1C7D |

___
Engineering mailing list
engineer...@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/engineering


signature.asc
Description: PGP signature
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Physikerwelt]

Hi Thiemo, hi all,

given my strong support for T78768 and the connection to this change,
I would like to express my regrets to the engineers blocked by the
change in Gerrit. As typical for strike actions, it disrupts the usual
business. However, I see the positive effect that people are made
aware of the fact that the code review process needs to be improved.

While I am happy with the code review duration of the patches I submit
today, I had problems to get the (obviously bad) code I wrote in the
beginning reviewed. I guess one of the main reasons for the
improvement is that I know the reviewers in person, today.

However, visiting Hackathons and annoying WMF employees in their
offices is not something that scales. Therefore, the review process
needs to be changed. After years of discussion and only little changes
to the process, this change brings this important but not urgent topic
to the agenda, which I appreciate.

To my experience Thiemo ist one of the most predictable (=good) code
reviewers. I am more than happy that he shared his process in his
email. I think his code reviewing procedure is exemplary and should
act as a general template. Thank you Thiemo.

One last hopefully constructive point. To my experience, the most
annoying experience in waiting for CR is when multiple reviewers are
requested and no feedback is provided. My wish would be that the state
of the patch is visualized in the (Gerrit) UI. So that the submitter
of the patch knows the state of the change and can estimate how long
it might take until the patch proceeds to the next stage.

Happy coding
physikerwelt


On Fri, Jan 18, 2019 at 1:25 PM MA  wrote:
>
> Hello,
>
> I agree with what Giuseppe Lavagetto and Jaime Crespo said.
>
> In my case, I am now getting review requests from several repos I
> contributed some time in the past, but for which I'm not a qualified
> reviewer. The plugin is also adding bots to review changes such as in
> .
>
> While I think it is certainly a good idea to help people find
> reviewers for their patches, I feel this plugin as it is now is going
> to achieve the contrary (mail blindness due to too many emails).
>
> I suggest we disable the plugin until at least Paladox's blacklist
> could be implemented and so we are given the choice to opt-out from
> it.
>
> Thank you, M.
>
> ___
> 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] Gerrit now automatically adds reviewers

[MA]

Hello,

I agree with what Giuseppe Lavagetto and Jaime Crespo said.

In my case, I am now getting review requests from several repos I
contributed some time in the past, but for which I'm not a qualified
reviewer. The plugin is also adding bots to review changes such as in
.

While I think it is certainly a good idea to help people find
reviewers for their patches, I feel this plugin as it is now is going
to achieve the contrary (mail blindness due to too many emails).

I suggest we disable the plugin until at least Paladox's blacklist
could be implemented and so we are given the choice to opt-out from
it.

Thank you, M.

___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Paladox via Wikitech-l]

 I’ve been working on a opt out (blacklist) in the plugin, see 
https://gerrit-review.googlesource.com/c/plugins/reviewers-by-blame/+/210812
On Friday, 18 January 2019, 09:39:55 GMT, Gergo Tisza 
 wrote:  
 
 On Thu, Jan 17, 2019 at 10:58 PM Giuseppe Lavagetto <
glavage...@wikimedia.org> wrote:

> While this gets improved, is there a way to opt-out from the feature
> individually or as a project?
>

Individually, no. On the project level, just set the max number of
reviewers added by the plugin to zero. (It seems that you need to use the
old Gerrit interface to see settings that come from plugins...)
___
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] Gerrit now automatically adds reviewers

[Gergo Tisza]

On Thu, Jan 17, 2019 at 10:58 PM Giuseppe Lavagetto <
glavage...@wikimedia.org> wrote:

> While this gets improved, is there a way to opt-out from the feature
> individually or as a project?
>

Individually, no. On the project level, just set the max number of
reviewers added by the plugin to zero. (It seems that you need to use the
old Gerrit interface to see settings that come from plugins...)
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Pine W]

Although this change doesn't personally affect me because I'm not active on
Gerrit, my reading of the situation is similar to Giuseppe's. Sending
people potentially large numbers of automatic and unsolicited notifications
is not something that I would generally support, and I would likely believe
to be a misuse of communications tools and an inconsiderate use of others'
time, no matter how well-intentioned.

My impression of this change to communications is that it should have been
broadly discussed, and consensus requested through an RfC, before
implementation. Was there an RfC? Were there previous wide notifications
regarding this proposed change to communications?

Pine
( https://meta.wikimedia.org/wiki/User:Pine )
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Re: [Wikitech-l] [Engineering] Gerrit now automatically adds reviewers

[Giuseppe Lavagetto]

On Thu, Jan 17, 2019 at 10:52 PM Greg Grossmeier  wrote:

> Hello,
>
> Yesterday we (the Release Engineering team) enabled a Gerrit plugin that
> will automatically add reviewers to your changes based on who previously
> has committed changes to the file.
>
>
While I commend the intention, this means I will get pinged for virtually
any change in a couple of very busy repositories.

The amount of noise will prevent me from being able to notice anyone's
review request. I think it's going to be the same for other developers - I
don't want to imagine what the inbox of a long-time mediawiki-core
contributor must look like!

What I fear is that the flood of reviews will make everyone just dull to
notifications, obtaining the exact opposite effect that was intended. I say
this because  I auto added myself to all reviews in operations/puppet[1] in
the past, which resulted in me ignoring all code review requests.

I think a good compromise would be to modify the plugin so that it adds
reviewers automatically, only if you're a new contributor (so you have -
say - less than N patches submitted).

While this gets improved, is there a way to opt-out from the feature
individually or as a project?

Thanks,
Giuseppe

[1] we already have a way to "monitor" all changes to a repository, to a
directory within a repository, or even to individual files, which I was
using extensively. Should we remove that?
-- 
Giuseppe Lavagetto
Principal Site Reliability Engineer, Wikimedia Foundation
___
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l