Re: [PATCH 1/2] drm/i915: PSR: Let's rely more on frontbuffer tracking.

Damien Lespiau
2015-11-19T12:03:51Z

On Thu, Nov 19, 2015 at 11:24:22AM +0000, Zanoni, Paulo R wrote:
> Em Qui, 2015-11-19 às 13:16 +0200, Jani Nikula escreveu:
> > On Wed, 18 Nov 2015, "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
> > wrote:
> > > Em Qua, 2015-11-18 às 11:21 -0800, Rodrigo Vivi escreveu:
> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > s/Cc/Reviewed-by/
> >
> > I don't think our patchwork is quite smart enough for that yet, so
> > for
> > future reference, please be write the whole thing.
>
> I completely forgot about this. Sorry.
>
> I guess that also means that if I say "If you do this and this and
> that, then I'll give you Reviewed-by: Paulo Zanoni <paulo.r.zanoni@inte
> l.com> it will mark the patch as reviewed even though it needs rework,
> right? I guess we just can't trust the PW automatic tag parsing since
> it can have either false positives or false negatives. Not until some
> major breakthroughs in AI.

The Reviewed-by parsing is quite simple: it will match '^Reviewed-by:'.
That means indenting the tag should be enough to bypass patchwork's
accounting.

That said, I think we need to extend the language we use for review and
make patchwork understand it. For instance, being able to review
several patches in one go could be done with:

Patches 1-3,6: Reviewed-by: Damien Lespiau damien.lespiau@intel.com

(See: https://github.com/dlespiau/patchwork/issues/27)

> Is suggesting deprecating the use of emails as a way to handle patches
> still considered trolling?

No :)

The way I see it is that it's easier to progressively enhance what we
have to do what we want -- I'll be the first to say patchwork is very
fragile today -- rather than switching to something totally different,
probably closing the door to non-intel contributors and suddenly having
two different systems for core DRM patch handling, among other things.

Either way, we have to try to improve what we have. I took one path but
it doesn't mean that was the best, someone else can always try to take
another set of tools and convince/force people to use that. The goals
are the important bit: test every patch before it's merged into
-nightly, improve the developer experience and patch flow/tracking
along the way.

For the first goal, we are almost there (in terms of patchwork, the CI
system, piglit, intel-gpu-tools, ... still need quite a bit for work):

For instance on series #890, I've uploaded checkpatch.pl test results:

http://patchwork.freedesktop.org/series/890/

I'll be deploying that checkpatch.pl test in the next week or so as an
example of patchwork/test integration. QA is looking into that
integration with BATs at the moment. Of course, email/notifications are
part of the plan once happy enough with the tests.

For the second goal, it's a long process of small improvements. We're
really not there, but interesting things have been created already: I'm
quite a fan of git-pw to retrieve series from the ml, for those series
correctly parsed that is...

Which leads me to the last thing: parsing things from the ml is fragile.
The main problems are both people and to a lesser extend mailing-lists:
using mails to carry patches does have problems, the vast majority of
problems come from people though. People will get stuff wrong: attach
patches to the wrong message-id, do things that are plain weird like
suddenly attaching patch "2.4/10" as a way to say "oh actually insert a
11th patch in the middle there", typo the review tags, ...

I think the last part is solvable, by having a tool wrapping git
send-email to do the right things, or at least deterministic things,
when sending patches and reviews. That's a bit blue sky stuff, I
certainly would love to get there eventually though.

Thinking about it, it's wouldn't actually be that complicated to have a
start of such a tool, I've captured the first simple thoughts here:
https://github.com/dlespiau/patchwork/issues/81

Oh well, it's a way longer email than the one I wanted to write.

HTH,

--
Damien