On Fri, Aug 16, 2013 at 10:54 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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 <dan.carpenter@xxxxxxxxxx> 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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel