Re: [PATCH V3 RESEND 8/8] staging: rtl8712: change SupportedRates to rates

[Dan Carpenter]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/