[Mesa-dev] [RFC] [PATCH 1/1] mesa: setup_glsl_generate_mipmap(): meta program compile failed

[Paul Berry]

On 4 October 2012 03:43, Oliver McFadden <oliver.mcfadden at linux.intel.com>wrote:

> On Wed, Oct 03, 2012 at 07:54:50PM -0700, Matt Turner wrote:
> > > mesa: setup_glsl_generate_mipmap(): meta program compile failed
> >
> > This is not a commit title. After reading the title, I should have at
> > least some idea of what the commit does. Pasting the error tells me
> > nothing.
>
> I'd include Mesa's error message describing _why_ the failure occurred,
> if it wasn't a bunch of corrupt bytes (another bug...)
>

I'm not sure if this is what Matt was getting at, but the difficulty I had
understanding your commit title was that I couldn't tell without reading
the commit whether (a) you were fixing a problem where an error failed to
be generated when it should, or (b) you were fixing a problem where an
error was generated when it shouldn't.  Also, I couldn't tell at a glance
which area of functionality you were affecting (mipmap generation) or what
you were doing to affect the error condition (changing the choice of which
shader to use).  Finally, the prefix "mesa:" misled me into thinking that
this patch affected core mesa (meta ops are not part of core mesa--they're
an optional component that is included by some but not all drivers).

FWIW, I usually try to make the commit title a verb phrase in present tense
that describes what the code now does that it didn't previously do, and the
circumstances under which the new behaviour manifests.  So for this patch
(as you've revised it, with the code motion removed), I'd recommend titling
it something like "meta: use proper shaders to generate mipmaps for GLES2".


>
> --
> Oliver McFadden.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20121004/76f2c7ff/attachment.html>