Gerrit is commonly used as a place to share works in progress early.
This is great, but it has an unfortunate side effect of making it harder for would-be reviewers to find patches that need reviewing using this query: https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co...
Could I ask that as a norm, if you post a WIP patch that you also self -2 it?
In addition to this, if a patch is open for longer than several months you may want to abandon it - it's much more useful to link to an abandoned patchset in a phabricator task which has all the context for someone who might be able to solve the problem it tries to solve.
I'd love to get us to a place where https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co... is manageable that code gets merged left right and center :)
Warm regards, Jon
On 12 May 2016 at 14:26, Jon Robson jdlrobson@gmail.com wrote:
Gerrit is commonly used as a place to share works in progress early.
This is great, but it has an unfortunate side effect of making it harder for would-be reviewers to find patches that need reviewing using this query:
https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co...
Could I ask that as a norm, if you post a WIP patch that you also self -2 it?
I disagree with this suggestion. The convention to date is having "WIP" or "DONTMERGE" in the git commit title. What's wrong with that?
I'd love to get us to a place where
https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co... is manageable that code gets merged left right and center :)
Use https://gerrit.wikimedia.org/r/#/q/(NOT+message:WIP)+AND+(NOT+message:DONTME... instead.
J.
I didn't know you could search message. Thanks. Even so.. the crux of what I'm asking for is a reliable way to filter these out. Even with the search you gave me I see "DONOTSUBMIT" in certain messages :)
On Thu, May 12, 2016 at 2:29 PM, James Forrester jforrester@wikimedia.org wrote:
On 12 May 2016 at 14:26, Jon Robson jdlrobson@gmail.com wrote:
Gerrit is commonly used as a place to share works in progress early.
This is great, but it has an unfortunate side effect of making it harder for would-be reviewers to find patches that need reviewing using this query:
https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co...
Could I ask that as a norm, if you post a WIP patch that you also self -2 it?
I disagree with this suggestion. The convention to date is having "WIP" or "DONTMERGE" in the git commit title. What's wrong with that?
I'd love to get us to a place where
https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co... is manageable that code gets merged left right and center :)
Use https://gerrit.wikimedia.org/r/#/q/(NOT+message:WIP)+AND+(NOT+message:DONTME... instead.
J.
James D. Forrester Lead Product Manager, Editing Wikimedia Foundation, Inc.
jforrester@wikimedia.org | @jdforrester _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
We definitely need consistency for any convention like this to be useful. Phabricator has the equivalent of self-2 in Differential: `arc diff --plan-changes`
It's a good convention, and I will try to adhere to the self-2 in addition to added WIP in the commit subject.
On Thu, May 12, 2016 at 4:44 PM, Jon Robson jdlrobson@gmail.com wrote:
I didn't know you could search message. Thanks. Even so.. the crux of what I'm asking for is a reliable way to filter these out. Even with the search you gave me I see "DONOTSUBMIT" in certain messages :)
On Thu, May 12, 2016 at 2:29 PM, James Forrester jforrester@wikimedia.org wrote:
On 12 May 2016 at 14:26, Jon Robson jdlrobson@gmail.com wrote:
Gerrit is commonly used as a place to share works in progress early.
This is great, but it has an unfortunate side effect of making it harder for would-be reviewers to find patches that need reviewing using this query:
https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co...
Could I ask that as a norm, if you post a WIP patch that you also self
-2
it?
I disagree with this suggestion. The convention to date is having "WIP"
or
"DONTMERGE" in the git commit title. What's wrong with that?
I'd love to get us to a place where
https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co...
is manageable that code gets merged left right and center :)
Use
https://gerrit.wikimedia.org/r/#/q/(NOT+message:WIP)+AND+(NOT+message:DONTME...
instead.
J.
James D. Forrester Lead Product Manager, Editing Wikimedia Foundation, Inc.
jforrester@wikimedia.org | @jdforrester _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
-- Jon Robson
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 12 May 2016 at 22:26, Jon Robson jdlrobson@gmail.com wrote:
Could I ask that as a norm, if you post a WIP patch that you also self -2 it?
I think you can only -2 if you have the rights necessary to +2?
Oh good point, you are correct, at least it would seem that way. I only have the option to -1 and +1 on operations/puppet, no -2
On Thu, May 12, 2016 at 5:12 PM, Alex Monk krenair@gmail.com wrote:
On 12 May 2016 at 22:26, Jon Robson jdlrobson@gmail.com wrote:
Could I ask that as a norm, if you post a WIP patch that you also self -2 it?
I think you can only -2 if you have the rights necessary to +2? _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Jon Robson jdlrobson@gmail.com wrote:
Gerrit is commonly used as a place to share works in progress early.
This is great, but it has an unfortunate side effect of making it harder for would-be reviewers to find patches that need reviewing using this query: https://gerrit.wikimedia.org/r/#/q/(NOT+label:Verified%253D-1)+AND+(label:Co...
Could I ask that as a norm, if you post a WIP patch that you also self -2 it?
[…]
No, -2 is restricted to project owners and thus not an op- tion for the vast majority of contributors. For that pur- pose, I proposed a Gerrit label "WIP" (cf. http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/840...).
Tim
Hi!
No, -2 is restricted to project owners and thus not an op- tion for the vast majority of contributors. For that pur- pose, I proposed a Gerrit label "WIP" (cf. http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/840...).
This looks like a nice solution.
. On 12 May 2016 4:18 p.m., "Stas Malyshev" smalyshev@wikimedia.org wrote:
Hi!
No, -2 is restricted to project owners and thus not an op- tion for the vast majority of contributors. For that pur- pose, I proposed a Gerrit label "WIP" (cf.
http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/840... ).
This looks like a nice solution.
Seconded. What's stopping us from adopting? It seems in that thread nothing happened.
Greg - is this something we can do?
-- Stas Malyshev smalyshev@wikimedia.org
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Gerrit also has drafts...
On 13 May 2016 at 01:56, Jon Robson jdlrobson@gmail.com wrote:
. On 12 May 2016 4:18 p.m., "Stas Malyshev" smalyshev@wikimedia.org wrote:
Hi!
No, -2 is restricted to project owners and thus not an op- tion for the vast majority of contributors. For that pur- pose, I proposed a Gerrit label "WIP" (cf.
http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/840... ).
This looks like a nice solution.
Seconded. What's stopping us from adopting? It seems in that thread nothing happened.
Greg - is this something we can do?
-- Stas Malyshev smalyshev@wikimedia.org
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
<quote name="Addshore" date="2016-05-13" time="16:48:47 +0100">
Gerrit also has drafts...
Drafts are only visible to the author, unfortunately. But yes, that'd work, but it's also fairly drastic. And people might not know about it before they push ('git-review .') and then they can't (afaict) make their change into a draft (per https://phabricator.wikimedia.org/T63124 ).
Upstream docs: https://gerrit-review.googlesource.com/Documentation/intro-user.html#drafts
I like the WIP status label better, personally :)
Greg
Am 13.05.2016 um 18:23 schrieb Greg Grossmeier:
<quote name="Addshore" date="2016-05-13" time="16:48:47 +0100"> > Gerrit also has drafts...
Drafts are only visible to the author, unfortunately.
And anyone the author adds as a reviewer. Works nicely in my experience.
On Wed, May 18, 2016 at 8:35 AM Daniel Kinzler daniel.kinzler@wikimedia.de wrote:
Am 13.05.2016 um 18:23 schrieb Greg Grossmeier:
<quote name="Addshore" date="2016-05-13" time="16:48:47 +0100"> > Gerrit also has drafts...
Drafts are only visible to the author, unfortunately.
And anyone the author adds as a reviewer. Works nicely in my experience.
Which I guess is nice if you really only want a specific group of people to see it, but it keeps the casual reviewer from chiming in.
This is why they seemed useful for security patches at first, except that they're disclosable over `git fetch` if you know where to look.
So yeah, I'm convinced they're defective by design as they're both too secret and not secret enough at the same time.
-Chad
On Wed, May 18, 2016 at 10:41 PM, Chad innocentkiller@gmail.com wrote:
On Wed, May 18, 2016 at 8:35 AM Daniel Kinzler daniel.kinzler@wikimedia.de wrote:
Am 13.05.2016 um 18:23 schrieb Greg Grossmeier:
<quote name="Addshore" date="2016-05-13" time="16:48:47 +0100"> > Gerrit also has drafts...
Drafts are only visible to the author, unfortunately.
And anyone the author adds as a reviewer. Works nicely in my experience.
Which I guess is nice if you really only want a specific group of people to see it, but it keeps the casual reviewer from chiming in.
This is why they seemed useful for security patches at first, except that they're disclosable over `git fetch` if you know where to look.
So yeah, I'm convinced they're defective by design as they're both too secret and not secret enough at the same time.
For non-security patches, discovery isnt a problem. They are cookie-licked bugs, and should already been discoverable via a link on a Phabricator task. If there is no Phabricator task, they are an unloved patch for an undefined problem. Hiding them as a draft un-licks them.
But that depends on https://phabricator.wikimedia.org/T63124 , and probably an upgrade of Gerrit, which will take forever because of the plan to switch to using Phabricator for code review.
<quote name="Jon Robson" date="2016-05-12" time="17:56:24 -0700">
. On 12 May 2016 4:18 p.m., "Stas Malyshev" smalyshev@wikimedia.org wrote:
Hi!
No, -2 is restricted to project owners and thus not an op- tion for the vast majority of contributors. For that pur- pose, I proposed a Gerrit label "WIP" (cf.
http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/840... ).
This looks like a nice solution.
Seconded. What's stopping us from adopting? It seems in that thread nothing happened.
Greg - is this something we can do?
Certainly.
"Adopting"? :) Making it official? I'd guess just setting the Gerrit config to exposing the WIP label and then documenting it on mw.org, maybe one of: * https://www.mediawiki.org/wiki/Gerrit/Advanced_usage * https://www.mediawiki.org/wiki/Gerrit/Code_review (only focused on the review part, not the submission part, should it expand in scope?) * https://www.mediawiki.org/wiki/Gerrit/Getting_started ? maybe too basic/focused to have this? * Maybe a new page describing the stages of a change and how to start contributing? I think of this because of this task:https://phabricator.wikimedia.org/T73357 - "Add a welcome bot to Gerrit for first time contributors"
Task to track this for Gerrit I've created: https://phabricator.wikimedia.org/T135245
(Below is just some documentation on how to do it that I wrote last night...)
How to do it via the command line?
For Gerrit, use what was proposed in that last thread by Tim L:
<quote name="Tim Landscheidt" date="2015-09-14" time="21:07:47 +0000">
"C. Scott Ananian" cananian@wikimedia.org wrote:
I'd use this tag more often if I could set it from the gerrit command-line when I upload a patch. Otherwise it will be pretty inconvenient to keep this in sync with the summary line of my patch.
That should be possible with Gerrit's command line inter- face (cf. https://gerrit-review.googlesource.com/Documentation/cmd-review.html). For example, I just voted on https://gerrit.wikimedia.org/r/#/c/238201/ with:
| ssh -p 29418 gerrit.wikimedia.org gerrit review --label Code-Review=+1 4266a950bf7d0984cc5177b0f2f8d76b7d0b3c55
This /should/ work for arbitrary labels.
For those experimenting with Differential already it's even easier per Mukunda:
<quote name="Mukunda Modell" date="2016-05-12" time="17:19:06 -0500">
We definitely need consistency for any convention like this to be useful. Phabricator has the equivalent of self-2 in Differential: `arc diff --plan-changes`
From arcanist's help information:
greg@x230 ~ % arc help diff <snip> --plan-changes Create or update a revision without requesting a code review. <snip>
Which makes something like this: https://secure.phabricator.com/D15906
Then that change does not show up in searches for Diffs needing review. :)
Best,
Greg
wikitech-l@lists.wikimedia.org