[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