Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Tue, Dec 05, 2017 at 01:40:02PM +0100, Marcus Wolf wrote: > > It's not the greatest, but it's not the worst... The configuration for > > ->enable_sync is a bit spread out and it might be nice to move it all to > > one function? > > > > I liked Simon's naming scheme and I thought it was clear

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Tue, Dec 05, 2017 at 01:40:02PM +0100, Marcus Wolf wrote: > > It's not the greatest, but it's not the worst... The configuration for > > ->enable_sync is a bit spread out and it might be nice to move it all to > > one function? > > > > I liked Simon's naming scheme and I thought it was clear

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 05.12.2017 um 13:16 schrieb Dan Carpenter: > On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: >> Keep in mind, that if you split the functions, in the interface >> implementation you also need more code: >> >> SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); >> >>

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 05.12.2017 um 13:16 schrieb Dan Carpenter: > On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: >> Keep in mind, that if you split the functions, in the interface >> implementation you also need more code: >> >> SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); >> >>

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: > Keep in mind, that if you split the functions, in the interface > implementation you also need more code: > > SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); > > will have to be converted in something like > > if

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: > Keep in mind, that if you split the functions, in the interface > implementation you also need more code: > > SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); > > will have to be converted in something like > > if

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 04.12.2017 um 21:05 schrieb Simon Sandström: > On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: >> >> Hi Simon, hi Dan, >> >> if you both are of the same opinion, for me, it's fine, if we go with two >> functions. >> >> But I don't get the advantage, if we split approx. 10

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 04.12.2017 um 21:05 schrieb Simon Sandström: > On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: >> >> Hi Simon, hi Dan, >> >> if you both are of the same opinion, for me, it's fine, if we go with two >> functions. >> >> But I don't get the advantage, if we split approx. 10

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: > > Hi Simon, hi Dan, > > if you both are of the same opinion, for me, it's fine, if we go with two > functions. > > But I don't get the advantage, if we split approx. 10 functions, to get rid > of enum optionOnOff. > > Keep in mind,

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: > > Hi Simon, hi Dan, > > if you both are of the same opinion, for me, it's fine, if we go with two > functions. > > But I don't get the advantage, if we split approx. 10 functions, to get rid > of enum optionOnOff. > > Keep in mind,

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 04.12.2017 um 21:42 schrieb Simon Sandström: On Mon, Dec 04, 2017 at 09:22:06PM +0200, Marcus Wolf wrote: Am 04.12.2017 um 21:15 schrieb Dan Carpenter: That's a bad name, because it doesn't just enable it also disables. Please split them. regards, dan carpenter Same applies to all

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 04.12.2017 um 21:42 schrieb Simon Sandström: On Mon, Dec 04, 2017 at 09:22:06PM +0200, Marcus Wolf wrote: Am 04.12.2017 um 21:15 schrieb Dan Carpenter: That's a bad name, because it doesn't just enable it also disables. Please split them. regards, dan carpenter Same applies to all

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 09:22:06PM +0200, Marcus Wolf wrote: > > > Am 04.12.2017 um 21:15 schrieb Dan Carpenter: > > > > That's a bad name, because it doesn't just enable it also disables. > > Please split them. > > > > regards, > > dan carpenter > > > > > > Same applies to all other stuff,

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 09:22:06PM +0200, Marcus Wolf wrote: > > > Am 04.12.2017 um 21:15 schrieb Dan Carpenter: > > > > That's a bad name, because it doesn't just enable it also disables. > > Please split them. > > > > regards, > > dan carpenter > > > > > > Same applies to all other stuff,

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 04.12.2017 um 21:15 schrieb Dan Carpenter: On Mon, Dec 04, 2017 at 08:37:51PM +0200, Marcus Wolf wrote: 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

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 04.12.2017 um 21:15 schrieb Dan Carpenter: On Mon, Dec 04, 2017 at 08:37:51PM +0200, Marcus Wolf wrote: 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

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 08:37:51PM +0200, Marcus Wolf wrote: > > > 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: > > > > > >

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Mon, Dec 04, 2017 at 08:37:51PM +0200, Marcus Wolf wrote: > > > 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: > > > > > >

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

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

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

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

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

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

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

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

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Sun, Dec 03, 2017 at 04:17:24PM +0100, Simon Sandström wrote: > Renames the enum optionOnOff and its values optionOn, optionOff to enum > option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings: > "Avoid CamelCase: , , ". > > Signed-off-by: Simon Sandström >

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Sun, Dec 03, 2017 at 04:17:24PM +0100, Simon Sandström wrote: > Renames the enum optionOnOff and its values optionOn, optionOff to enum > option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings: > "Avoid CamelCase: , , ". > > Signed-off-by: Simon Sandström > --- >

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Sun, Dec 03, 2017 at 06:49:40PM +0200, Marcus Wolf wrote: > > Hi Simon, Hi, > > thanks for your effort. > > I have two questions: > * According to my practical experiance, enums were always written in lower > case. Does kernel style guide ask for upper case for enums? Yes. From

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

On Sun, Dec 03, 2017 at 06:49:40PM +0200, Marcus Wolf wrote: > > Hi Simon, Hi, > > thanks for your effort. > > I have two questions: > * According to my practical experiance, enums were always written in lower > case. Does kernel style guide ask for upper case for enums? Yes. From

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 03.12.2017 um 17:17 schrieb Simon Sandström: Renames the enum optionOnOff and its values optionOn, optionOff to enum option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings: "Avoid CamelCase: , , ". Signed-off-by: Simon Sandström ---

Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Am 03.12.2017 um 17:17 schrieb Simon Sandström: Renames the enum optionOnOff and its values optionOn, optionOff to enum option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings: "Avoid CamelCase: , , ". Signed-off-by: Simon Sandström --- drivers/staging/pi433/pi433_if.c | 34

[PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Renames the enum optionOnOff and its values optionOn, optionOff to enum option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings: "Avoid CamelCase: , , ". Signed-off-by: Simon Sandström --- drivers/staging/pi433/pi433_if.c | 34 ++---

[PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h

Renames the enum optionOnOff and its values optionOn, optionOff to enum option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings: "Avoid CamelCase: , , ". Signed-off-by: Simon Sandström --- drivers/staging/pi433/pi433_if.c | 34 ++---