Re: Using Gerrit drafts

[Daniel Hecht]

I haven't looked at your edits. I was responding to Phil's comment
that I quoted.

On Thu, Oct 19, 2017 at 1:08 PM, Jim Apple  wrote:
> I'm surprised my edits gave you the impression that I think people should
> not review their code before asking for peer review.
>
> On Thu, Oct 19, 2017 at 1:06 PM, Daniel Hecht  wrote:
>
>> I think it's a good idea for everyone to review their own code before
>> asking for a peer review.  A lot of style nits, etc can be addressed
>> before the first peer review iteration that way.
>>
>> On Thu, Oct 19, 2017 at 11:44 AM, Philip Zeyliger 
>> wrote:
>> 
>> > I think it's a good idea for new contributors to review their code
>> reviews
>> > first and explicitly hit publish. Experienced contributors will get
>> > sufficiently acquainted with Gerrit over time.
>> >
>>

Re: Using Gerrit drafts

[Jim Apple]

I'm surprised my edits gave you the impression that I think people should
not review their code before asking for peer review.

On Thu, Oct 19, 2017 at 1:06 PM, Daniel Hecht  wrote:

> I think it's a good idea for everyone to review their own code before
> asking for a peer review.  A lot of style nits, etc can be addressed
> before the first peer review iteration that way.
>
> On Thu, Oct 19, 2017 at 11:44 AM, Philip Zeyliger 
> wrote:
> 
> > I think it's a good idea for new contributors to review their code
> reviews
> > first and explicitly hit publish. Experienced contributors will get
> > sufficiently acquainted with Gerrit over time.
> >
>

Re: Using Gerrit drafts

[Daniel Hecht]

I think it's a good idea for everyone to review their own code before
asking for a peer review.  A lot of style nits, etc can be addressed
before the first peer review iteration that way.

On Thu, Oct 19, 2017 at 11:44 AM, Philip Zeyliger  wrote:

> I think it's a good idea for new contributors to review their code reviews
> first and explicitly hit publish. Experienced contributors will get
> sufficiently acquainted with Gerrit over time.
>

Re: Using Gerrit drafts

[Jim Apple]

Sorry, didn't mean "workflow", mean "repo".

On Thu, Oct 19, 2017 at 11:54 AM, Jim Apple  wrote:

> In between when I saw the edit and when I saw this email, I re-edited.
> Please fell free to re-edit again. Mostly I didn't want "Publish" to sound
> like it was going to be part of the workflow, but I also made drafts the
> second suggestion, because I would expect gerrit newbies to publish there
> too often and then disappear when nobody reviews their code.
>
> New users misunderstanding and abusing refs/for/master is rare.
>
> On Thu, Oct 19, 2017 at 11:44 AM, Philip Zeyliger 
> wrote:
>
>> On Thu, Oct 19, 2017 at 10:50 AM, Daniel Hecht 
>> wrote:
>>
>> > Add this info to
>> > https://cwiki.apache.org/confluence/display/IMPALA/
>> > Using+Gerrit+to+submit+and+review+patches
>> > if not already there?
>>
>>
>> Thanks for the suggestion.
>>
>> I updated
>> https://cwiki.apache.org/confluence/display/IMPALA/Using+
>> Gerrit+to+submit+and+review+patches#UsingGerrittosubmitandreviewpatches-
>> Sendingapatchforreview.1
>> to basically recommend the draft workflow over the "refs/for" workflow. I
>> think it's a good idea for new contributors to review their code reviews
>> first and explicitly hit publish. Experienced contributors will get
>> sufficiently acquainted with Gerrit over time.
>>
>> -- Philip
>>
>
>

Re: Using Gerrit drafts

[Jim Apple]

In between when I saw the edit and when I saw this email, I re-edited.
Please fell free to re-edit again. Mostly I didn't want "Publish" to sound
like it was going to be part of the workflow, but I also made drafts the
second suggestion, because I would expect gerrit newbies to publish there
too often and then disappear when nobody reviews their code.

New users misunderstanding and abusing refs/for/master is rare.

On Thu, Oct 19, 2017 at 11:44 AM, Philip Zeyliger 
wrote:

> On Thu, Oct 19, 2017 at 10:50 AM, Daniel Hecht 
> wrote:
>
> > Add this info to
> > https://cwiki.apache.org/confluence/display/IMPALA/
> > Using+Gerrit+to+submit+and+review+patches
> > if not already there?
>
>
> Thanks for the suggestion.
>
> I updated
> https://cwiki.apache.org/confluence/display/IMPALA/
> Using+Gerrit+to+submit+and+review+patches#UsingGerrittosubmitandreviewpa
> tches-Sendingapatchforreview.1
> to basically recommend the draft workflow over the "refs/for" workflow. I
> think it's a good idea for new contributors to review their code reviews
> first and explicitly hit publish. Experienced contributors will get
> sufficiently acquainted with Gerrit over time.
>
> -- Philip
>

Re: Using Gerrit drafts

[Philip Zeyliger]

On Thu, Oct 19, 2017 at 10:50 AM, Daniel Hecht  wrote:

> Add this info to
> https://cwiki.apache.org/confluence/display/IMPALA/
> Using+Gerrit+to+submit+and+review+patches
> if not already there?


Thanks for the suggestion.

I updated
https://cwiki.apache.org/confluence/display/IMPALA/Using+Gerrit+to+submit+and+review+patches#UsingGerrittosubmitandreviewpatches-Sendingapatchforreview.1
to basically recommend the draft workflow over the "refs/for" workflow. I
think it's a good idea for new contributors to review their code reviews
first and explicitly hit publish. Experienced contributors will get
sufficiently acquainted with Gerrit over time.

-- Philip

Re: Using Gerrit drafts

[Philip Zeyliger]

Gerrit is replacing them with something called "private changes".
https://gerrit-review.googlesource.com/Documentation/intro-user.html#private-changes.
They look equivalent to me, with slightly different syntax.

On Thu, Oct 19, 2017 at 11:26 AM, John Russell 
wrote:

> I had a problem accessing my existing drafts after a recent upgrade to
> gerrit.  Somehow after the upgrade, it became much more important to use
> the ‘ssh’ method instead of ‘http’ in that pulldown where you see the
> Download / Pull / etc. commands.  In the course of resolving that problem,
> I received this nugget of info:
>
> > The current Gerrit installation is making some changes in behavior to
> how drafts behave, and as I read the documentation, draft functionality is
> going away entirely soon.
>
> Let me follow up a little bit on that.  I thought it meant the gerrit
> project itself was doing away with drafts, but maybe it’s a decision about
> that site that we could push back on.
>
> John
>
> > On Oct 19, 2017, at 9:34 AM, Philip Zeyliger 
> wrote:
> >
> > Hey folks,
> >
> > This wasn't obvious for me, so I figured I'd share it. If you want to
> > review your Gerrit changes on the Gerrit UI before sending e-mail to the
> > community, you can run something like:
> >
> > git push asf-gerrit HEAD:refs/drafts/master
> >
> > This will give you a URL that you can browse to, and you can even run
> > https://jenkins.impala.io/view/Utility/job/pre-review-test/ against it.
> No
> > e-mails are sent!
> >
> > Once you've looked it over, you can hit 'Publish' on the web UI, and,
> boom,
> > e-mails.
> >
> > Cheers,
> >
> > -- Philip
>
>

Re: Using Gerrit drafts

[John Russell]

I had a problem accessing my existing drafts after a recent upgrade to gerrit.  
Somehow after the upgrade, it became much more important to use the ‘ssh’ 
method instead of ‘http’ in that pulldown where you see the Download / Pull / 
etc. commands.  In the course of resolving that problem, I received this nugget 
of info:

> The current Gerrit installation is making some changes in behavior to how 
> drafts behave, and as I read the documentation, draft functionality is going 
> away entirely soon. 

Let me follow up a little bit on that.  I thought it meant the gerrit project 
itself was doing away with drafts, but maybe it’s a decision about that site 
that we could push back on.

John

> On Oct 19, 2017, at 9:34 AM, Philip Zeyliger  wrote:
> 
> Hey folks,
> 
> This wasn't obvious for me, so I figured I'd share it. If you want to
> review your Gerrit changes on the Gerrit UI before sending e-mail to the
> community, you can run something like:
> 
> git push asf-gerrit HEAD:refs/drafts/master
> 
> This will give you a URL that you can browse to, and you can even run
> https://jenkins.impala.io/view/Utility/job/pre-review-test/ against it. No
> e-mails are sent!
> 
> Once you've looked it over, you can hit 'Publish' on the web UI, and, boom,
> e-mails.
> 
> Cheers,
> 
> -- Philip

Re: Using Gerrit drafts

[Daniel Hecht]

Add this info to
https://cwiki.apache.org/confluence/display/IMPALA/Using+Gerrit+to+submit+and+review+patches
if not already there?

On Thu, Oct 19, 2017 at 10:41 AM, Tim Armstrong  wrote:
> I thought subsequent drafts were only visible to reviewers (in general
> drafts are visible to any reviewers you add to the patch). At least on
> older versions of gerrit if you pushed out a draft to a published patchset,
> a notification email was sent out but the updated patchset was invisible to
> non-reviewers.
>
> Another feature I find useful for organising related patches is the
> "topic". If you push to refs/for/master%topic=buffer-pool then the patch is
> associated with a topic.
>
> On Thu, Oct 19, 2017 at 9:50 AM, Lars Volker  wrote:
>
>> Note, that publishing cannot be undone. In particular, after you published
>> a change, subsequent pushes to refs/drafts will be public patch sets, too.
>>
>> On Oct 19, 2017 09:34, "Philip Zeyliger"  wrote:
>>
>> Hey folks,
>>
>> This wasn't obvious for me, so I figured I'd share it. If you want to
>> review your Gerrit changes on the Gerrit UI before sending e-mail to the
>> community, you can run something like:
>>
>> git push asf-gerrit HEAD:refs/drafts/master
>>
>> This will give you a URL that you can browse to, and you can even run
>> https://jenkins.impala.io/view/Utility/job/pre-review-test/ against it. No
>> e-mails are sent!
>>
>> Once you've looked it over, you can hit 'Publish' on the web UI, and, boom,
>> e-mails.
>>
>> Cheers,
>>
>> -- Philip
>>

Re: Using Gerrit drafts

[Tim Armstrong]

I thought subsequent drafts were only visible to reviewers (in general
drafts are visible to any reviewers you add to the patch). At least on
older versions of gerrit if you pushed out a draft to a published patchset,
a notification email was sent out but the updated patchset was invisible to
non-reviewers.

Another feature I find useful for organising related patches is the
"topic". If you push to refs/for/master%topic=buffer-pool then the patch is
associated with a topic.

On Thu, Oct 19, 2017 at 9:50 AM, Lars Volker  wrote:

> Note, that publishing cannot be undone. In particular, after you published
> a change, subsequent pushes to refs/drafts will be public patch sets, too.
>
> On Oct 19, 2017 09:34, "Philip Zeyliger"  wrote:
>
> Hey folks,
>
> This wasn't obvious for me, so I figured I'd share it. If you want to
> review your Gerrit changes on the Gerrit UI before sending e-mail to the
> community, you can run something like:
>
> git push asf-gerrit HEAD:refs/drafts/master
>
> This will give you a URL that you can browse to, and you can even run
> https://jenkins.impala.io/view/Utility/job/pre-review-test/ against it. No
> e-mails are sent!
>
> Once you've looked it over, you can hit 'Publish' on the web UI, and, boom,
> e-mails.
>
> Cheers,
>
> -- Philip
>

Re: Using Gerrit drafts

[Lars Volker]

Note, that publishing cannot be undone. In particular, after you published
a change, subsequent pushes to refs/drafts will be public patch sets, too.

On Oct 19, 2017 09:34, "Philip Zeyliger"  wrote:

Hey folks,

This wasn't obvious for me, so I figured I'd share it. If you want to
review your Gerrit changes on the Gerrit UI before sending e-mail to the
community, you can run something like:

git push asf-gerrit HEAD:refs/drafts/master

This will give you a URL that you can browse to, and you can even run
https://jenkins.impala.io/view/Utility/job/pre-review-test/ against it. No
e-mails are sent!

Once you've looked it over, you can hit 'Publish' on the web UI, and, boom,
e-mails.

Cheers,

-- Philip

Using Gerrit drafts

[Philip Zeyliger]

Hey folks,

This wasn't obvious for me, so I figured I'd share it. If you want to
review your Gerrit changes on the Gerrit UI before sending e-mail to the
community, you can run something like:

git push asf-gerrit HEAD:refs/drafts/master

This will give you a URL that you can browse to, and you can even run
https://jenkins.impala.io/view/Utility/job/pre-review-test/ against it. No
e-mails are sent!

Once you've looked it over, you can hit 'Publish' on the web UI, and, boom,
e-mails.

Cheers,

-- Philip