On Wed, Oct 26, 2016 at 11:52:41AM +0300, Dan Carpenter wrote:
> So, when I'm reviewing these, there are obviously a lot of patches that
> go through staging and I try to do them as fast as possible and quite
> often make mistakes. Feel free to speak up when that happens, I won't
> be offended.
>
> I use a script to review them which strips our all the white space
> changes because I don't care about those really. I mostly care about
> bugs.
>
> The problem is that this patch removes some extra curly braces:
>
> if (!(PEEK32(PANEL_PLL_CTRL) & PLL_CTRL_POWER) ||
> - !(PEEK32(PANEL_DISPLAY_CTRL) & DISPLAY_CTRL_TIMING)) {
> + !(PEEK32(PANEL_DISPLAY_CTRL) & DISPLAY_CTRL_TIMING))
> return;
> - }
>
> If the patch had *just* deleted the tabs, this would have been basically
> a 20 second review.
>
> Anyway, I've reviewed it now and it's fine. But next time, please don't
> mix those kinds of white space changes. It seems tiny but it messes
> with my scripts.
>
> regards,
> dan carpenter
>
I thought about breaking the patch in two to separate the braces
removal, but decided to keep only one patch because this tiny change
seemed harmless. Didn't know it could cause such trouble, but I'll
keep this in mind for future patches.
thank you,
elise
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
