[Intel-gfx] [PATCH][v2 1/2] drm/i915: prepare for video codec ring buffer on Sandybridge

[Xiang, Haihao]

On Mon, 2010-09-13 at 17:52 +0800, Chris Wilson wrote:
> 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. 

Thanks for your comments. I will separate it into three patches

Haihao

>     
>     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
>