From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751714AbbG1WI3 (ORCPT ); Tue, 28 Jul 2015 18:08:29 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:20781 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751512AbbG1WI1 (ORCPT ); Tue, 28 Jul 2015 18:08:27 -0400 Date: Wed, 29 Jul 2015 01:07:58 +0300 From: Dan Carpenter To: Joshua Clayton Cc: Julia Lawall , devel@driverdev.osuosl.org, Florian Schilhabel , Greg Kroah-Hartman , Nitin Kuppelur , linux-kernel@vger.kernel.org, Sudip Mukherjee , Larry Finger Subject: Re: [PATCH V3 RESEND 8/8] staging: rtl8712: change SupportedRates to rates Message-ID: <20150728220758.GB5096@mwanda> References: <9594685.hpOiZGHKrk@jclayton-pc> <20150728155638.GE5180@mwanda> <11512207.BLTbrXTcg1@jclayton-pc> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <11512207.BLTbrXTcg1@jclayton-pc> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 28, 2015 at 10:49:52AM -0700, Joshua Clayton wrote: > > Changing the line breaks here is a tiny change on the same line and so > > it's fine. It fits into the one thing per patch rule. > > This is the style I prefer (getting rid of the explicit == true) > > - if ((r8712_is_cckratesonly_included(pnetwork->network. > - SupportedRates)) == true) { > + if (r8712_is_cckratesonly_included(pnetwork->network.rates)) { > if (ht_cap == true) > snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bn"); > else > snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11b"); > - } else if ((r8712_is_cckrates_included(pnetwork->network. > - SupportedRates)) == true) { > + } else if (r8712_is_cckrates_included(pnetwork->network.rates)) { > if (ht_cap == true) > snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bgn"); > > Does that look ok? Yes. It looks better. > If we keep the "== true" and the extra set of parentheses, the "else if" case goes over 80 lines. > I will happily submit the change as follow up patch if that is too much to change at once. The "one thing per patch" rule is a fuzzy line. My scripts don't care about white space very much so moving the .rates to the other line is not a big deal. Also the line break was really bad and the == true is only mildly untidy at worst. If it's a massive patch then removing the == true makes things more difficult, yes, but if it's small then it's probably fine to do it at once since it's on the same line and makes the 80 character rule work. What I'm saying is that just use your best judgement what is easy to review and we try to be reasonable as well. regards, dan carpenter