[Intel-gfx] [PATCH][v2 1/2] drm/i915: prepare for video codec ring buffer on Sandybridge
Chris Wilson
chris at chris-wilson.co.uk
Mon Sep 13 11:52:02 CEST 2010
On Mon, 13 Sep 2010 15:17:05 +0800, "Xiang, Haihao" <haihao.xiang at intel.com> wrote:
> Some little changes:
> Add set_tail hook to struct intel_ring_buffer
> fix HAS_BSD with a device info flag
> Don't export the initialiser of struct intel_ring_buffer
A really nice set of cleanups, thanks! However, that changelog should have
been an instant give away that something was wrong with the patch. ;-)
Carl, would you care to remind us how to write a good commit? You do it so
much better than I. Here is my lame version:
- A patch should just do one thing and one thing only.
This is vital if we ever need to bisect or revert a patch. It also
means that we create smaller, more readable commits - which is a good
thing!
- Give an overview of what was done and more importantly *why*.
We can all read code and spend a long time pondering the complexities
and mysteries of a piece of code and eventually come to an
understanding of what that code does. We will never be able to work
out what you were thinking or intending to do as you wrote that piece
of code though.
You may have to write several paragraphs explaining the background and
your analysis of a bug or design that you wish to implement.
Obviously, for these simple cleanups there is little to say other than
it makes the code easier to read and reduces the chance for subtle
bugs to creep in. More complex code requires deeper thought and
understanding and the changelog should reflect that.
- The patch should record all those who contributed to the discovery of
the bug, if applicable, and to those who reviewed and tested the
patches. If the patch touches code outside of our sole purview, we
must obtain at least an ACK by the maintainer of that code.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx
mailing list