Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752326AbdLDSiC (ORCPT <rfc822;w@1wt.eu>);
        Mon, 4 Dec 2017 13:38:02 -0500
Received: from dd39320.kasserver.com ([85.13.155.146]:55626 "EHLO
        dd39320.kasserver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751570AbdLDSiB (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 4 Dec 2017 13:38:01 -0500
Subject: Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in
 rf69_enum.h
To: Dan Carpenter <dan.carpenter@oracle.com>,
        =?UTF-8?Q?Simon_Sandstr=c3=b6m?= <simon@nikanor.nu>
Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
        linux@Wolf-Entwicklungen.de, linux-kernel@vger.kernel.org
References: <20171203151726.16639-1-simon@nikanor.nu>
 <20171203151726.16639-5-simon@nikanor.nu>
 <20171204101737.xv4eyglsrbmiagtn@mwanda>
 <20171204103719.7talcb3dqu3yfns2@mwanda>
From: Marcus Wolf <marcus.wolf@smarthome-wolf.de>
Message-ID: <401fb0b7-48a9-dfc9-481b-d4e31f0ed9b5@smarthome-wolf.de>
Date: Mon, 4 Dec 2017 20:37:51 +0200
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <20171204103719.7talcb3dqu3yfns2@mwanda>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: de-DE
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1347
Lines: 44



Am 04.12.2017 um 12:37 schrieb Dan Carpenter:
> On Mon, Dec 04, 2017 at 01:17:37PM +0300, Dan Carpenter wrote:
>> Perhaps choose different function names if you want?  You could do it
>> as several patches:
>>
>> patch 1: change types to bool
>> patch 2: sed -e '/ == optionOn//'
>> patch 3: split the functions into two functions
>> patch 4: delete optionOnOff enum
>>
>> patches 1 and 2 could be merged together (your choice).
>>
> 
> Markus says that optionOn is used by user space so my you won't be able
> to remove these entirely.  But as much as possible we should internally.
> 
> regards,
> dan carpenter
> 

Hi Dan, hi Simon,

I think, it's a pretty nice idea to remove th optionOnOff and replace it 
by bool.

<history>
In former times, the variables in the config struct had very different 
names - not containing "enable". Therefore optionOnOff was used to make 
absolutely clear (in user space), wheter something was switched on, or off.
Now the variable have nice names, so bool is fine, even better now :-)
</history>

I would suggest not to split the amp-functions but to rename them, to 
also contain an enable:
rf69_set_amp_X_enable()

To avoid misunderstandings maybe it is better to remove the enable from 
enable_address_filtering, since here we can't go with bool.

Thanks a lot for the ideas and improvements :-)

Marcus