[Piglit] Weekly 10 Picks from Patchwork for review and friendly reminder to clean out your old patches

[Timothy Arceri]

On Fri, 2015-06-19 at 21:12 +0100, Jose Fonseca wrote:
> On 19/06/15 20:45, Ilia Mirkin wrote:
> > On Fri, Jun 19, 2015 at 3:39 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
> >> On 19/06/15 13:32, Timothy Arceri wrote:
> >>>
> >>> Hi all,
> >>>
> >>> Unfortunately since its introduction patchwork hasn't seen a lot of love
> >>> in the Piglit and Mesa projects so I thought I'd try something out to
> >>> bring it out of the shadows and into the limelight.
> >>>
> >>> The idea is simple we have many useful but long forgotten patches
> >>> sitting on the mailing list that would serve us much better sitting in
> >>> the git repo, so once a week I (or anyone else that wants to help out)
> >>> would pick 10 seemingly random older patches that could do with a
> >>> review/update/etc.
> >>>
> >>> I'm hoping this will help with both clearing out the backlog of patches
> >>> and getting people thinking about patchwork.
> >>>
> >>> I'm interested in feedback on what people think about this idea.
> >>
> >>
> >> Patchwork seems to fail to recognize submited patches.  Eg. one of my
> >> patches is https://patchwork.freedesktop.org/patch/51379/ but it has been
> >> commited on
> >> http://cgit.freedesktop.org/piglit/commit/?id=540972b46e51ee1d4acbb3757b731a066e2b6ba5
> >>
> >> Why is that?
> >
> > It's very strict about matching patches. The diff has to be identical.
> > Which it often isn't if you made minor changes, or rebased, or
> > whatever.
> 
> Without a bit of fuzzy matching I'm afraid this looks a bit hopeless to me:

Sure its not perfect but isn't it better than nothing, if you remember
to take a look every now an then the effort is minimal. If someone was
really keen they could always add this feature to the upstream project
if it doesn't exist already.

> 
> I believe the bulk of the patches are committed, and only a few is 
> forgotten. 

Sure I closed a bunch of both superseded and committed patches when
picking these out yesterday. But this doesn't change the fact that
patches are being forgotten, we are always quick to suggest people write
piglit tests or extend the tests they have written but we are not taking
advantage of the work that already been done, the effort seems minimal
vs the result. 

Of the tests I've come across so far a lot of them have been for bugs
that were fixed in Mesa, shouldn't we be concerned about that.

I obviously can't give numbers but I picked out 10 non patch series
patches (besides the first one) from the first 2 pages (of 12).
So if I use that as an average that's at least 60 single patches
forgotten.
It's even more concerning that patch series for a whole features are
sitting there. 

>  Looking at the patchwork backlog it's fair to say a large 
> portion of those committed don't get detected due to small changes.  So 
> the end result is that developers have to click through and babysit the 
> bulk of their changes in patchwork, so that the few truly forgotten 
> patches get to stand out?

Yes but as Ilia said if you have admin access you can bulk close (you
may be able to do this for your own patches). If everyone did it once
every couple of weeks it wouldn't take much effort.

> 
> I don't think this will ever going to work.  There's no incentive in the 
> system for the most prolific developers to spend so much of their time, 
> for the sake of the occasional contributor.  The patchwork system seems 
> bound to echo what happens on the mailing list: their patches get to be 
> lost twice...

As I said I don't believe its much effort its just a small change in
workflow. Personally I find patchwork a good way to keep track of the
changes I've sent out so I don't forget about them.
If you look at the list I created none of these patches are from
occasional contributors.

> 
> 
> There 's another concern -- one can only change the status of our own 
> patches.  So if one commits on behalf of somebody else, and that patch 
> doesn't get recognized, one needs to get that other person to register 
> and click through patchwork?
> 

I'm not sure who we have to ask to get admin, but once you have that you
can bulk close.


As for all your suggestions below they sound good and I agree it's the
future but I was just trying to improve the situation with the tools we
have now :)

> 
> 
> 
> 
> I wonder if it wouldn't be better to have a more comprehensive solution 
> for review and tracking, ala github pull requests.   Maybe have an 
> official mirror for mesa/piglit in github, or deploy gitlab 
> (https://about.gitlab.com/features/) in fdo.org, or something along 
> those lines, and start tracking this sort of things as pull requests.
> 
> I known it might look (and be) a wild idea at the moment, but I believe 
> this will be the future eventually: with things like cloud-based CI 
> systems (Travis CI, AppVeyor), projects can have testsuites run 
> automatically on pull requests (No GPU HW available, but one can still 
> ensure builds don't fail, run unit tests, and even rendering tests with 
> SW renderers) and detect issues even before reviewing or committing.
> 
> I've seen this happen first-handed: I once make a pull request to an 
> open-source project I had never contributed on github, a few minutes 
> later bot added a comment saying that the project built fine and all 
> unit tests passed, and all the maintainer had to do was clicking a button.
> 
> I'm now trying to repro this on some of my open source projects. (E.g, 
> Apitrace). I still have a long way to go, but already it is showing 
> fruits -- I immediately know when a Linux developr proposes a Apitrace 
> change that breaks Windows vuild (or a Windows developer breaks Linux 
> build) , and I can point them to the logs and they can often fix them 
> selves.  I hope one day I have unit tests and more there too.
> 
> 
> Jose