Review promise :-)

Review promise :-)

[Carlos O'Donell]

Increasingly I'm actually turning to patchwork to look for patches to
review. I'm trying to structure my work day around an eventual attempt
to use Patchwork 2.0.

For example I was looking for patches from H.J. to review and I
cleaned up his list of owned patches in patchwork to track down what
still needed review and committing.

This was actually quite a nice way to work and is the kind of workflow
I wanted to have in place with gerritt but this works with email.

The hard part is that each developers queue is messy because we don't
keep them clean.

So here is my promise to *anyone*.

If you as a developer keep your patchwork queue clean.

If you ask me to look at your patches and you tell me your patchwork
queue is clean.

I will review at least one patch off that queue *immediately*.

Cheers,
Carlos.

Re: Review promise :-)

[Florian Weimer]

* Carlos O'Donell:

> If you as a developer keep your patchwork queue clean.

I thought that we are going to throw away all that data soon?
Or is that not going to happen?

Thanks,
Florian

Re: Review promise :-)

[Carlos O'Donell]

On Thu, Feb 13, 2020 at 10:04 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Carlos O'Donell:
>
> > If you as a developer keep your patchwork queue clean.
>
> I thought that we are going to throw away all that data soon?
> Or is that not going to happen?

Siddhesh,

Correct me if I'm wrong but you were able to migrate the data to Patchwork 2.0?

I thought you said you had the ability to migrate the data?

Florian,

Even if we throw the data out... you need to decide if this is worth
my review promise ;-)

This is incentive for me to do more reviews too!

Cheers,
Carlos.

Re: Review promise :-)

[Siddhesh Poyarekar]

On 13/02/20 20:36, Carlos O'Donell wrote:
> Correct me if I'm wrong but you were able to migrate the data to Patchwork 2.0?
> 
> I thought you said you had the ability to migrate the data?

In my last test I got all the way to 1.1 and got stuck there; this as on
the osci box that Sergio gave me access to.  The django migration
scripts broke at that point and I gave up to do other things.

I'll try to find some time to give it another shot this weekend.  I've
promised to look at the ChangeLog generation slowness too, so patchwork
goes below that in priority.

Siddhesh

Re: Review promise :-)

[Siddhesh Poyarekar]

On 13/02/20 21:14, Siddhesh Poyarekar wrote:
> On 13/02/20 20:36, Carlos O'Donell wrote:
>> Correct me if I'm wrong but you were able to migrate the data to Patchwork 2.0?
>>
>> I thought you said you had the ability to migrate the data?
> 
> In my last test I got all the way to 1.1 and got stuck there; this as on
> the osci box that Sergio gave me access to.  The django migration
> scripts broke at that point and I gave up to do other things.
> 
> I'll try to find some time to give it another shot this weekend.  I've
> promised to look at the ChangeLog generation slowness too, so patchwork
> goes below that in priority.

I realized I didn't actually answer your question completely: yes the
intent is to migrate all data and at the moment it doesn't look
impossible.  The script breakage is not a showstopper IIRC, so we're
still on target to achieving a proper migration with data intact.

Siddhesh

Re: Review promise :-)

[Zack Weinberg]

On Thu, Feb 13, 2020 at 9:57 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> Increasingly I'm actually turning to patchwork to look for patches to
> review. I'm trying to structure my work day around an eventual attempt
> to use Patchwork 2.0.
>
> For example I was looking for patches from H.J. to review and I
> cleaned up his list of owned patches in patchwork to track down what
> still needed review and committing.
>
> This was actually quite a nice way to work and is the kind of workflow
> I wanted to have in place with gerritt but this works with email.
>
> The hard part is that each developers queue is messy because we don't
> keep them clean.

Is there a way to operate on lots of patches at once?  My queue is
full of old patches that should be marked either committed or dropped,
it would be nice to not have to load the individual patch page for
each one and push the same buttons for each.

zw

Re: Review promise :-)

[Florian Weimer]

* Zack Weinberg:

> On Thu, Feb 13, 2020 at 9:57 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> Increasingly I'm actually turning to patchwork to look for patches to
>> review. I'm trying to structure my work day around an eventual attempt
>> to use Patchwork 2.0.
>>
>> For example I was looking for patches from H.J. to review and I
>> cleaned up his list of owned patches in patchwork to track down what
>> still needed review and committing.
>>
>> This was actually quite a nice way to work and is the kind of workflow
>> I wanted to have in place with gerritt but this works with email.
>>
>> The hard part is that each developers queue is messy because we don't
>> keep them clean.
>
> Is there a way to operate on lots of patches at once?  My queue is
> full of old patches that should be marked either committed or dropped,
> it would be nice to not have to load the individual patch page for
> each one and push the same buttons for each.

After logging in, I see checkboxes on the left here:

  <https://patchwork.sourceware.org/project/glibc/list/?submitter=73>

This enables bulk updates for me.

Thanks,
Florian

Re: Review promise :-)

[Carlos O'Donell]

On Thu, Feb 13, 2020 at 11:01 AM Zack Weinberg <zackw@panix.com> wrote:
>
> On Thu, Feb 13, 2020 at 9:57 AM Carlos O'Donell <carlos@redhat.com> wrote:
> >
> > Increasingly I'm actually turning to patchwork to look for patches to
> > review. I'm trying to structure my work day around an eventual attempt
> > to use Patchwork 2.0.
> >
> > For example I was looking for patches from H.J. to review and I
> > cleaned up his list of owned patches in patchwork to track down what
> > still needed review and committing.
> >
> > This was actually quite a nice way to work and is the kind of workflow
> > I wanted to have in place with gerritt but this works with email.
> >
> > The hard part is that each developers queue is messy because we don't
> > keep them clean.
>
> Is there a way to operate on lots of patches at once?  My queue is
> full of old patches that should be marked either committed or dropped,
> it would be nice to not have to load the individual patch page for
> each one and push the same buttons for each.

Yeah, absolutely.

You have two options:
* Script it using the pwclient CLI
* Use the web interface, apply a filter, click the top "Patch"
checkmark to check all selected, then apply a property and it is
applied to all the selected patches.

Cheers,
Carlos.

Re: Review promise :-)

[Zack Weinberg]

On Thu, Feb 13, 2020 at 11:19 AM Carlos O'Donell <carlos@redhat.com> wrote:
> On Thu, Feb 13, 2020 at 11:01 AM Zack Weinberg <zackw@panix.com> wrote:
> >
> > Is there a way to operate on lots of patches at once?  My queue is
> > full of old patches that should be marked either committed or dropped,
> > it would be nice to not have to load the individual patch page for
> > each one and push the same buttons for each.
>
> Yeah, absolutely.
>
> You have two options:
> * Script it using the pwclient CLI
> * Use the web interface, apply a filter, click the top "Patch"
> checkmark to check all selected, then apply a property and it is
> applied to all the selected patches.

I see the checkboxes in the web interface, but I don't see any
controls for taking actions on the selected patches?  Yes, I'm logged
in.

zw

Re: Review promise :-)

[Florian Weimer]

* Zack Weinberg:

> On Thu, Feb 13, 2020 at 11:19 AM Carlos O'Donell <carlos@redhat.com> wrote:
>> On Thu, Feb 13, 2020 at 11:01 AM Zack Weinberg <zackw@panix.com> wrote:
>> >
>> > Is there a way to operate on lots of patches at once?  My queue is
>> > full of old patches that should be marked either committed or dropped,
>> > it would be nice to not have to load the individual patch page for
>> > each one and push the same buttons for each.
>>
>> Yeah, absolutely.
>>
>> You have two options:
>> * Script it using the pwclient CLI
>> * Use the web interface, apply a filter, click the top "Patch"
>> checkmark to check all selected, then apply a property and it is
>> applied to all the selected patches.
>
> I see the checkboxes in the web interface, but I don't see any
> controls for taking actions on the selected patches?  Yes, I'm logged
> in.

For me, there's a Properties box at the end of the page.

Thanks,
Florian

Re: Review promise :-)

[Zack Weinberg]

On Thu, Feb 13, 2020 at 12:58 PM Florian Weimer <fweimer@redhat.com> wrote:
> * Zack Weinberg:
> > On Thu, Feb 13, 2020 at 11:19 AM Carlos O'Donell <carlos@redhat.com> wrote:
> >> On Thu, Feb 13, 2020 at 11:01 AM Zack Weinberg <zackw@panix.com> wrote:
> >> >
> >> > Is there a way to operate on lots of patches at once?  My queue is
> >> > full of old patches that should be marked either committed or dropped,
> >> > it would be nice to not have to load the individual patch page for
> >> > each one and push the same buttons for each.
> >>
> >> Yeah, absolutely.
> >>
> >> You have two options:
> >> * Script it using the pwclient CLI
> >> * Use the web interface, apply a filter, click the top "Patch"
> >> checkmark to check all selected, then apply a property and it is
> >> applied to all the selected patches.
> >
> > I see the checkboxes in the web interface, but I don't see any
> > controls for taking actions on the selected patches?  Yes, I'm logged
> > in.
>
> For me, there's a Properties box at the end of the page.

Weird.  I see a "Bundling" box, but no Properties box.

zw

Re: Review promise :-)

[Carlos O'Donell]

On Thu, Feb 13, 2020 at 1:15 PM Zack Weinberg <zackw@panix.com> wrote:
>
> On Thu, Feb 13, 2020 at 1:12 PM Zack Weinberg <zackw@panix.com> wrote:
> > On Thu, Feb 13, 2020 at 12:58 PM Florian Weimer <fweimer@redhat.com> wrote:
> > > * Zack Weinberg:
> > > > On Thu, Feb 13, 2020 at 11:19 AM Carlos O'Donell <carlos@redhat.com> wrote:
> > > >> On Thu, Feb 13, 2020 at 11:01 AM Zack Weinberg <zackw@panix.com> wrote:
> > > >> >
> > > >> > Is there a way to operate on lots of patches at once?  My queue is
> > > >> > full of old patches that should be marked either committed or dropped,
> > > >> > it would be nice to not have to load the individual patch page for
> > > >> > each one and push the same buttons for each.
> > > >>
> > > >> Yeah, absolutely.
> > > >>
> > > >> You have two options:
> > > >> * Script it using the pwclient CLI
> > > >> * Use the web interface, apply a filter, click the top "Patch"
> > > >> checkmark to check all selected, then apply a property and it is
> > > >> applied to all the selected patches.
> > > >
> > > > I see the checkboxes in the web interface, but I don't see any
> > > > controls for taking actions on the selected patches?  Yes, I'm logged
> > > > in.
> > >
> > > For me, there's a Properties box at the end of the page.
> >
> > Weird.  I see a "Bundling" box, but no Properties box.
>
> To avoid further round-trips, here is a screenshot of everything I can
> see on a search results page.  (My actual queue is three pages long,
> this is a very small subset.)

Fixed.

You need to be a listed project maintainer to be able to edit patch status.

I've updated MAINTAINERS to ensure future developers ask for this permission.

https://sourceware.org/glibc/wiki/MAINTAINERS

Cheers,
Carlos.

Re: Review promise :-)

[Zack Weinberg]

On Thu, Feb 13, 2020 at 1:21 PM Carlos O'Donell <carlos@redhat.com> wrote:
> > To avoid further round-trips, here is a screenshot of everything I can
> > see on a search results page.  (My actual queue is three pages long,
> > this is a very small subset.)
>
> Fixed.
>
> You need to be a listed project maintainer to be able to edit patch status.

Thanks, it works now.  (It's a little odd that _bulk_ edits require
this privilege but edits on individual patches don't, but that's
probably something to take up with patchwork upstream.)

zw

Re: Review promise :-)

[Joseph Myers]

On Thu, 13 Feb 2020, Carlos O'Donell wrote:

> The hard part is that each developers queue is messy because we don't
> keep them clean.

One benefit we were supposed to get from stopping manual ChangeLog edits 
was that the git-patch-id of a commit for a patch committed with no 
changes would match the git-patch-id of the patch posted to the mailing 
list - and so patchwork would be able to mark such a patch as committed 
automatically.

I'm not clear if this is a standard patchwork feature or just something we 
hoped to be able to implement after stopping manual ChangeLog edits, 
however.

(It might make sense to clean up existing patches in patchwork based on 
having a git-patch-id matching a change that is now on master, as well as 
doing such cleanup automatically in future.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Review promise :-)

[Carlos O'Donell]

On 2/13/20 3:54 PM, Joseph Myers wrote:
> On Thu, 13 Feb 2020, Carlos O'Donell wrote:
> 
>> The hard part is that each developers queue is messy because we don't
>> keep them clean.
> 
> One benefit we were supposed to get from stopping manual ChangeLog edits 
> was that the git-patch-id of a commit for a patch committed with no 
> changes would match the git-patch-id of the patch posted to the mailing 
> list - and so patchwork would be able to mark such a patch as committed 
> automatically.
> 
> I'm not clear if this is a standard patchwork feature or just something we 
> hoped to be able to implement after stopping manual ChangeLog edits, 
> however.
> 
> (It might make sense to clean up existing patches in patchwork based on 
> having a git-patch-id matching a change that is now on master, as well as 
> doing such cleanup automatically in future.)

I think that automating this is the goal with Patchwork 2.0.

Right now I'm walking developer patches queues from most recent patch
to oldest patch usually looking for something to review. I'm trying to
use patchwork in the way it was intended to see what the workflow would
look like.

-- 
Cheers,
Carlos.

Re: Review promise :-)

[Stefan Liebler]

On 2/13/20 7:31 PM, Zack Weinberg wrote:
> On Thu, Feb 13, 2020 at 1:21 PM Carlos O'Donell <carlos@redhat.com> wrote:
>>> To avoid further round-trips, here is a screenshot of everything I can
>>> see on a search results page.  (My actual queue is three pages long,
>>> this is a very small subset.)
>>
>> Fixed.
>>
>> You need to be a listed project maintainer to be able to edit patch status.
> 
> Thanks, it works now.  (It's a little odd that _bulk_ edits require
> this privilege but edits on individual patches don't, but that's
> probably something to take up with patchwork upstream.)
> 
> zw
> 

Carlos,

Can you please help. I see the checkboxes, but it seems as I only have 
the option to "Create bundle" at the bottom of the page or change each 
single patch.

Its not clear to me where I have to be listed?
- https://sourceware.org/glibc/wiki/MAINTAINERS
(here I am listed)
- https://patchwork.sourceware.org/project/glibc/
(here I am not listed in the maintainers box. Is this the missing piece? 
If yes, can you please add me.)

Another hint:
I had created my patchwork account with stli@linux.vnet.ibm.com .
In the meantime I use stli@linux.ibm.com .
I've just also linked stli@linux.ibm.com to the patchwork account.

Bye,
Stefan

Re: Review promise :-)

[Carlos O'Donell]

On 2/14/20 3:43 AM, Stefan Liebler wrote:
> On 2/13/20 7:31 PM, Zack Weinberg wrote:
>> On Thu, Feb 13, 2020 at 1:21 PM Carlos O'Donell <carlos@redhat.com> wrote:
>>>> To avoid further round-trips, here is a screenshot of everything I can
>>>> see on a search results page.  (My actual queue is three pages long,
>>>> this is a very small subset.)
>>>
>>> Fixed.
>>>
>>> You need to be a listed project maintainer to be able to edit patch status.
>>
>> Thanks, it works now.  (It's a little odd that _bulk_ edits require
>> this privilege but edits on individual patches don't, but that's
>> probably something to take up with patchwork upstream.)
>>
>> zw
>>
> 
> Carlos,
> 
> Can you please help. I see the checkboxes, but it seems as I only have the option to "Create bundle" at the bottom of the page or change each single patch.
> 
> Its not clear to me where I have to be listed?
> - https://sourceware.org/glibc/wiki/MAINTAINERS
> (here I am listed)
> - https://patchwork.sourceware.org/project/glibc/
> (here I am not listed in the maintainers box. Is this the missing piece? If yes, can you please add me.)
> 
> Another hint:
> I had created my patchwork account with stli@linux.vnet.ibm.com .
> In the meantime I use stli@linux.ibm.com .
> I've just also linked stli@linux.ibm.com to the patchwork account.

I just added you as a maintainer. Should be fixed now. Tell me if it's not.

-- 
Cheers,
Carlos.

Re: Review promise :-)

[Stefan Liebler]

On 2/14/20 5:48 PM, Carlos O'Donell wrote:
> On 2/14/20 3:43 AM, Stefan Liebler wrote:
>> On 2/13/20 7:31 PM, Zack Weinberg wrote:
>>> On Thu, Feb 13, 2020 at 1:21 PM Carlos O'Donell <carlos@redhat.com> wrote:
>>>>> To avoid further round-trips, here is a screenshot of everything I can
>>>>> see on a search results page.  (My actual queue is three pages long,
>>>>> this is a very small subset.)
>>>>
>>>> Fixed.
>>>>
>>>> You need to be a listed project maintainer to be able to edit patch status.
>>>
>>> Thanks, it works now.  (It's a little odd that _bulk_ edits require
>>> this privilege but edits on individual patches don't, but that's
>>> probably something to take up with patchwork upstream.)
>>>
>>> zw
>>>
>>
>> Carlos,
>>
>> Can you please help. I see the checkboxes, but it seems as I only have the option to "Create bundle" at the bottom of the page or change each single patch.
>>
>> Its not clear to me where I have to be listed?
>> - https://sourceware.org/glibc/wiki/MAINTAINERS
>> (here I am listed)
>> - https://patchwork.sourceware.org/project/glibc/
>> (here I am not listed in the maintainers box. Is this the missing piece? If yes, can you please add me.)
>>
>> Another hint:
>> I had created my patchwork account with stli@linux.vnet.ibm.com .
>> In the meantime I use stli@linux.ibm.com .
>> I've just also linked stli@linux.ibm.com to the patchwork account.
> 
> I just added you as a maintainer. Should be fixed now. Tell me if it's not.
> 
Thanks. It now works for me.