Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753902Ab3HPLE2 (ORCPT ); Fri, 16 Aug 2013 07:04:28 -0400 Received: from mail-vc0-f179.google.com ([209.85.220.179]:59756 "EHLO mail-vc0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753722Ab3HPLEZ (ORCPT ); Fri, 16 Aug 2013 07:04:25 -0400 MIME-Version: 1.0 In-Reply-To: <20130816085419.GD26086@mwanda> References: <1376595295-7820-1-git-send-email-jfrederich@gmail.com> <20130816071338.GC26086@mwanda> <20130816085419.GD26086@mwanda> Date: Fri, 16 Aug 2013 13:04:24 +0200 Message-ID: Subject: Re: [PATCH 2/3] Staging: olpc_dcon: replace some magic numbers From: Jens Frederich To: Dan Carpenter Cc: devel@driverdev.osuosl.org, Greg KH , Jon Nettleton , Daniel Drake , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2582 Lines: 61 On Fri, Aug 16, 2013 at 10:54 AM, Dan Carpenter wrote: > On Fri, Aug 16, 2013 at 09:40:38AM +0200, Jens Frederich wrote: >> On Fri, Aug 16, 2013 at 9:13 AM, Dan Carpenter wrote: >> > On Thu, Aug 15, 2013 at 09:34:55PM +0200, Jens Frederich wrote: >> >> The 0x42 initialize squence 0x101 is wrong. According to >> >> the specification Bit 8 is reserved, thus not in use. >> >> I removed it. >> > >> > Really these code changes should be in a separate patch and labeled >> > "Don't set reserved bit." instead of hidden away inside a cleanup >> > patch. >> > >> >> The patch is applied. Still, good to know. It's not so easy to find the >> right patch granularity as newbie. >> > > Yeah. Staging is for educating people about kernel process as much > as it is about writing code. > Yeah, I know it. Isn't easy to dive in the kernel process. I'm writing lots of in house code (vector.com), but the development process is very different. > The rule here is "Don't mix code changes into a cleanup patches." > What we want is if you have a bug then you can look through > `git log --oneline` output and guess which patch introduced the bug. > This patch is a cleanup patch so it shouldn't introduce any code > changes or any bugs. > > Meanwhile, if you are making a code change you can make any cleanups > you need to in order to do the change. Also if there is an existing > checkpatch warning on any of the lines you touch, then that's ok to > fix as well. Or if there are tiny related changes than that's fine. > > There are three problems with big patches: > 1) It breaks the --oneline summary to mix two things into one patch. > 2) It makes the patch harder to review. For example, sometimes > people fix a bug and rename 10 variables as well. > 3) The more lines your patch is, the more chance there is that we > will reject it based on one of those lines. You don't like > redoing patches and we don't like making people redo them. So > small patches are better and put the more controversial ones at > the end so the first patches can be applied. > > No one totally agrees what "small closely related cleanups" means so > it's better to be conservative. > Thanks for the detail information, you're right. I know it, it's a general problem. thanks, jens -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/