driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: comedi: Simplify conditional evaluation
@ 2020-10-18 19:48 Deepak R Varma
  2020-10-18 19:49 ` [PATCH 2/2] staging: comedi: combine split lines for improved readability Deepak R Varma
  2020-10-19 10:17 ` [PATCH 1/2] staging: comedi: Simplify conditional evaluation Ian Abbott
  0 siblings, 2 replies; 10+ messages in thread
From: Deepak R Varma @ 2020-10-18 19:48 UTC (permalink / raw)
  To: outreachy-kernel, Greg Kroah-Hartman, Ian Abbott,
	H Hartley Sweeten, devel
  Cc: mh12gx2825

Boolean comparison of the condition inside unittest function is
unnecessary and can be simplified by directly using the condition
outcome for evaluation. Issue reported by :
scripts/coccinelle/misc/boolinit.cocci

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
index eaefaf596a37..7db83cf5e4aa 100644
--- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
+++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
@@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
 	const struct ni_route_tables *T = &private.routing_tables;
 
 	init_pci_fake();
-	unittest(route_register_is_valid(4, O(4), T) == false,
+	unittest(!route_register_is_valid(4, O(4), T),
 		 "check for bad source 4-->4\n");
-	unittest(route_register_is_valid(0, O(1), T) == true,
+	unittest(!route_register_is_valid(0, O(1), T),
 		 "find first source\n");
-	unittest(route_register_is_valid(4, O(6), T) == true,
+	unittest(!route_register_is_valid(4, O(6), T),
 		 "find middle source\n");
-	unittest(route_register_is_valid(9, O(8), T) == true,
+	unittest(!route_register_is_valid(9, O(8), T),
 		 "find last source");
 }
 
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-18 19:48 [PATCH 1/2] staging: comedi: Simplify conditional evaluation Deepak R Varma
@ 2020-10-18 19:49 ` Deepak R Varma
  2020-10-19 10:22   ` Ian Abbott
  2020-10-19 10:17 ` [PATCH 1/2] staging: comedi: Simplify conditional evaluation Ian Abbott
  1 sibling, 1 reply; 10+ messages in thread
From: Deepak R Varma @ 2020-10-18 19:49 UTC (permalink / raw)
  To: outreachy-kernel, Greg Kroah-Hartman, Ian Abbott,
	H Hartley Sweeten, devel
  Cc: mh12gx2825

Instructions split on multiple lines can be combined on a single line
for improved readability of the code.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
index 7db83cf5e4aa..a3b1be623861 100644
--- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
+++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
@@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
 	const struct ni_route_tables *T = &private.routing_tables;
 
 	init_pci_fake();
-	unittest(!route_register_is_valid(4, O(4), T),
-		 "check for bad source 4-->4\n");
-	unittest(!route_register_is_valid(0, O(1), T),
-		 "find first source\n");
-	unittest(!route_register_is_valid(4, O(6), T),
-		 "find middle source\n");
-	unittest(!route_register_is_valid(9, O(8), T),
-		 "find last source");
+	unittest(!route_register_is_valid(4, O(4), T), "check for bad source 4-->4\n");
+	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
+	unittest(!route_register_is_valid(4, O(6), T), "find middle source\n");
+	unittest(!route_register_is_valid(9, O(8), T), "find last source");
 }
 
 void test_ni_check_trigger_arg(void)
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] staging: comedi: Simplify conditional evaluation
  2020-10-18 19:48 [PATCH 1/2] staging: comedi: Simplify conditional evaluation Deepak R Varma
  2020-10-18 19:49 ` [PATCH 2/2] staging: comedi: combine split lines for improved readability Deepak R Varma
@ 2020-10-19 10:17 ` Ian Abbott
  2020-10-19 10:51   ` Deepak R Varma
  1 sibling, 1 reply; 10+ messages in thread
From: Ian Abbott @ 2020-10-19 10:17 UTC (permalink / raw)
  To: Deepak R Varma, outreachy-kernel, Greg Kroah-Hartman,
	H Hartley Sweeten, devel

On 18/10/2020 20:48, Deepak R Varma wrote:
> Boolean comparison of the condition inside unittest function is
> unnecessary and can be simplified by directly using the condition
> outcome for evaluation. Issue reported by :
> scripts/coccinelle/misc/boolinit.cocci
> 
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>   drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> index eaefaf596a37..7db83cf5e4aa 100644
> --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> @@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
>   	const struct ni_route_tables *T = &private.routing_tables;
>   
>   	init_pci_fake();
> -	unittest(route_register_is_valid(4, O(4), T) == false,
> +	unittest(!route_register_is_valid(4, O(4), T),
>   		 "check for bad source 4-->4\n");
> -	unittest(route_register_is_valid(0, O(1), T) == true,
> +	unittest(!route_register_is_valid(0, O(1), T),
>   		 "find first source\n");
> -	unittest(route_register_is_valid(4, O(6), T) == true,
> +	unittest(!route_register_is_valid(4, O(6), T),
>   		 "find middle source\n");
> -	unittest(route_register_is_valid(9, O(8), T) == true,
> +	unittest(!route_register_is_valid(9, O(8), T),
>   		 "find last source");
>   }

NAK.

It looks like you have inadvertently inverted some of the tests.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-18 19:49 ` [PATCH 2/2] staging: comedi: combine split lines for improved readability Deepak R Varma
@ 2020-10-19 10:22   ` Ian Abbott
  2020-10-19 10:41     ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Abbott @ 2020-10-19 10:22 UTC (permalink / raw)
  To: Deepak R Varma, outreachy-kernel, Greg Kroah-Hartman,
	H Hartley Sweeten, devel

On 18/10/2020 20:49, Deepak R Varma wrote:
> Instructions split on multiple lines can be combined on a single line
> for improved readability of the code.
> 
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>   .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> index 7db83cf5e4aa..a3b1be623861 100644
> --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
>   	const struct ni_route_tables *T = &private.routing_tables;
>   
>   	init_pci_fake();
> -	unittest(!route_register_is_valid(4, O(4), T),
> -		 "check for bad source 4-->4\n");
> -	unittest(!route_register_is_valid(0, O(1), T),
> -		 "find first source\n");
> -	unittest(!route_register_is_valid(4, O(6), T),
> -		 "find middle source\n");
> -	unittest(!route_register_is_valid(9, O(8), T),
> -		 "find last source");
> +	unittest(!route_register_is_valid(4, O(4), T), "check for bad source 4-->4\n");
> +	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
> +	unittest(!route_register_is_valid(4, O(6), T), "find middle source\n");
> +	unittest(!route_register_is_valid(9, O(8), T), "find last source");
>   }
>   
>   void test_ni_check_trigger_arg(void)
> 

Is it worth breaking the 80-column limit for this?

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-19 10:22   ` Ian Abbott
@ 2020-10-19 10:41     ` Julia Lawall
  2020-10-19 10:57       ` Deepak R Varma
  0 siblings, 1 reply; 10+ messages in thread
From: Julia Lawall @ 2020-10-19 10:41 UTC (permalink / raw)
  To: Ian Abbott; +Cc: devel, outreachy-kernel, Deepak R Varma, Greg Kroah-Hartman



On Mon, 19 Oct 2020, Ian Abbott wrote:

> On 18/10/2020 20:49, Deepak R Varma wrote:
> > Instructions split on multiple lines can be combined on a single line
> > for improved readability of the code.
> >
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > ---
> >   .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
> >   1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > index 7db83cf5e4aa..a3b1be623861 100644
> > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
> >   	const struct ni_route_tables *T = &private.routing_tables;
> >     	init_pci_fake();
> > -	unittest(!route_register_is_valid(4, O(4), T),
> > -		 "check for bad source 4-->4\n");
> > -	unittest(!route_register_is_valid(0, O(1), T),
> > -		 "find first source\n");
> > -	unittest(!route_register_is_valid(4, O(6), T),
> > -		 "find middle source\n");
> > -	unittest(!route_register_is_valid(9, O(8), T),
> > -		 "find last source");
> > +	unittest(!route_register_is_valid(4, O(4), T), "check for bad source
> > 4-->4\n");
> > +	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
> > +	unittest(!route_register_is_valid(4, O(6), T), "find middle
> > source\n");
> > +	unittest(!route_register_is_valid(9, O(8), T), "find last source");
> >   }
> >     void test_ni_check_trigger_arg(void)
> >
>
> Is it worth breaking the 80-column limit for this?

Deepak,

It was much nicer before.

It can be awkward to break eg a + operation at the 80 character limit.
But function argument stand by themselves.

julia

>
> --
> -=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
> -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/f81a537c-c0fb-5133-52a3-825128814435%40mev.co.uk.
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] staging: comedi: Simplify conditional evaluation
  2020-10-19 10:17 ` [PATCH 1/2] staging: comedi: Simplify conditional evaluation Ian Abbott
@ 2020-10-19 10:51   ` Deepak R Varma
  0 siblings, 0 replies; 10+ messages in thread
From: Deepak R Varma @ 2020-10-19 10:51 UTC (permalink / raw)
  To: Ian Abbott; +Cc: devel, outreachy-kernel, Greg Kroah-Hartman

On Mon, Oct 19, 2020 at 11:17:38AM +0100, Ian Abbott wrote:
> On 18/10/2020 20:48, Deepak R Varma wrote:
> > Boolean comparison of the condition inside unittest function is
> > unnecessary and can be simplified by directly using the condition
> > outcome for evaluation. Issue reported by :
> > scripts/coccinelle/misc/boolinit.cocci
> > 
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > ---
> >   drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > index eaefaf596a37..7db83cf5e4aa 100644
> > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > @@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
> >   	const struct ni_route_tables *T = &private.routing_tables;
> >   	init_pci_fake();
> > -	unittest(route_register_is_valid(4, O(4), T) == false,
> > +	unittest(!route_register_is_valid(4, O(4), T),
> >   		 "check for bad source 4-->4\n");
> > -	unittest(route_register_is_valid(0, O(1), T) == true,
> > +	unittest(!route_register_is_valid(0, O(1), T),
> >   		 "find first source\n");
> > -	unittest(route_register_is_valid(4, O(6), T) == true,
> > +	unittest(!route_register_is_valid(4, O(6), T),
> >   		 "find middle source\n");
> > -	unittest(route_register_is_valid(9, O(8), T) == true,
> > +	unittest(!route_register_is_valid(9, O(8), T),
> >   		 "find last source");
> >   }
> 
> NAK.
> 
> It looks like you have inadvertently inverted some of the tests.

Hi Ian,
Thank you for catching that. I am correcting it and will send a v2
shortly.

Deepak.

> 
> -- 
> -=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
> -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-19 10:41     ` [Outreachy kernel] " Julia Lawall
@ 2020-10-19 10:57       ` Deepak R Varma
  2020-10-19 11:34         ` Ian Abbott
  0 siblings, 1 reply; 10+ messages in thread
From: Deepak R Varma @ 2020-10-19 10:57 UTC (permalink / raw)
  To: Julia Lawall; +Cc: devel, outreachy-kernel, Ian Abbott, Greg Kroah-Hartman

On Mon, Oct 19, 2020 at 12:41:14PM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 19 Oct 2020, Ian Abbott wrote:
> 
> > On 18/10/2020 20:49, Deepak R Varma wrote:
> > > Instructions split on multiple lines can be combined on a single line
> > > for improved readability of the code.
> > >
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > ---
> > >   .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
> > >   1 file changed, 4 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > index 7db83cf5e4aa..a3b1be623861 100644
> > > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
> > >   	const struct ni_route_tables *T = &private.routing_tables;
> > >     	init_pci_fake();
> > > -	unittest(!route_register_is_valid(4, O(4), T),
> > > -		 "check for bad source 4-->4\n");
> > > -	unittest(!route_register_is_valid(0, O(1), T),
> > > -		 "find first source\n");
> > > -	unittest(!route_register_is_valid(4, O(6), T),
> > > -		 "find middle source\n");
> > > -	unittest(!route_register_is_valid(9, O(8), T),
> > > -		 "find last source");
> > > +	unittest(!route_register_is_valid(4, O(4), T), "check for bad source
> > > 4-->4\n");
> > > +	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
> > > +	unittest(!route_register_is_valid(4, O(6), T), "find middle
> > > source\n");
> > > +	unittest(!route_register_is_valid(9, O(8), T), "find last source");
> > >   }
> > >     void test_ni_check_trigger_arg(void)
> > >
> >
> > Is it worth breaking the 80-column limit for this?
> 
> Deepak,
> 
> It was much nicer before.
> 
> It can be awkward to break eg a + operation at the 80 character limit.
> But function argument stand by themselves.
> 
> julia
> 

Hi Julia and Ian,
I wanted to take advantage of the relaxation of 80 column limit to 100
columns and hence proposed combining the lines. Are you saying this is
allowed only in certain cases?

Please confirm and I will handle it accordingly.

Thank you,
Deepak.

> >
> > --
> > -=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
> > -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> > -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> > -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/outreachy-kernel/f81a537c-c0fb-5133-52a3-825128814435%40mev.co.uk.
> >
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-19 10:57       ` Deepak R Varma
@ 2020-10-19 11:34         ` Ian Abbott
  2020-10-19 11:49           ` Deepak R Varma
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Abbott @ 2020-10-19 11:34 UTC (permalink / raw)
  To: Deepak R Varma, Julia Lawall; +Cc: devel, outreachy-kernel, Greg Kroah-Hartman

On 19/10/2020 11:57, Deepak R Varma wrote:
> On Mon, Oct 19, 2020 at 12:41:14PM +0200, Julia Lawall wrote:
>>
>>
>> On Mon, 19 Oct 2020, Ian Abbott wrote:
>>
>>> On 18/10/2020 20:49, Deepak R Varma wrote:
>>>> Instructions split on multiple lines can be combined on a single line
>>>> for improved readability of the code.
>>>>
>>>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>>>> ---
>>>>    .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
>>>>    1 file changed, 4 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>> b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>> index 7db83cf5e4aa..a3b1be623861 100644
>>>> --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>> +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>> @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
>>>>    	const struct ni_route_tables *T = &private.routing_tables;
>>>>      	init_pci_fake();
>>>> -	unittest(!route_register_is_valid(4, O(4), T),
>>>> -		 "check for bad source 4-->4\n");
>>>> -	unittest(!route_register_is_valid(0, O(1), T),
>>>> -		 "find first source\n");
>>>> -	unittest(!route_register_is_valid(4, O(6), T),
>>>> -		 "find middle source\n");
>>>> -	unittest(!route_register_is_valid(9, O(8), T),
>>>> -		 "find last source");
>>>> +	unittest(!route_register_is_valid(4, O(4), T), "check for bad source
>>>> 4-->4\n");
>>>> +	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
>>>> +	unittest(!route_register_is_valid(4, O(6), T), "find middle
>>>> source\n");
>>>> +	unittest(!route_register_is_valid(9, O(8), T), "find last source");
>>>>    }
>>>>      void test_ni_check_trigger_arg(void)
>>>>
>>>
>>> Is it worth breaking the 80-column limit for this?
>>
>> Deepak,
>>
>> It was much nicer before.
>>
>> It can be awkward to break eg a + operation at the 80 character limit.
>> But function argument stand by themselves.
>>
>> julia
>>
> 
> Hi Julia and Ian,
> I wanted to take advantage of the relaxation of 80 column limit to 100
> columns and hence proposed combining the lines. Are you saying this is
> allowed only in certain cases?
> 
> Please confirm and I will handle it accordingly.

Hi Deepak,

80 columns is still the preferred limit.  I think the relaxation is 
mostly to avoid the need to split sub-expressions across lines in really 
ugly ways to keep within the 80 columns at the expense of readability.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-19 11:34         ` Ian Abbott
@ 2020-10-19 11:49           ` Deepak R Varma
  2020-10-19 12:46             ` Ian Abbott
  0 siblings, 1 reply; 10+ messages in thread
From: Deepak R Varma @ 2020-10-19 11:49 UTC (permalink / raw)
  To: Ian Abbott; +Cc: Julia Lawall, outreachy-kernel, devel, Greg Kroah-Hartman

On Mon, Oct 19, 2020 at 12:34:15PM +0100, Ian Abbott wrote:
> On 19/10/2020 11:57, Deepak R Varma wrote:
> > On Mon, Oct 19, 2020 at 12:41:14PM +0200, Julia Lawall wrote:
> > > 
> > > 
> > > On Mon, 19 Oct 2020, Ian Abbott wrote:
> > > 
> > > > On 18/10/2020 20:49, Deepak R Varma wrote:
> > > > > Instructions split on multiple lines can be combined on a single line
> > > > > for improved readability of the code.
> > > > > 
> > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > ---
> > > > >    .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
> > > > >    1 file changed, 4 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > index 7db83cf5e4aa..a3b1be623861 100644
> > > > > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
> > > > >    	const struct ni_route_tables *T = &private.routing_tables;
> > > > >      	init_pci_fake();
> > > > > -	unittest(!route_register_is_valid(4, O(4), T),
> > > > > -		 "check for bad source 4-->4\n");
> > > > > -	unittest(!route_register_is_valid(0, O(1), T),
> > > > > -		 "find first source\n");
> > > > > -	unittest(!route_register_is_valid(4, O(6), T),
> > > > > -		 "find middle source\n");
> > > > > -	unittest(!route_register_is_valid(9, O(8), T),
> > > > > -		 "find last source");
> > > > > +	unittest(!route_register_is_valid(4, O(4), T), "check for bad source
> > > > > 4-->4\n");
> > > > > +	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
> > > > > +	unittest(!route_register_is_valid(4, O(6), T), "find middle
> > > > > source\n");
> > > > > +	unittest(!route_register_is_valid(9, O(8), T), "find last source");
> > > > >    }
> > > > >      void test_ni_check_trigger_arg(void)
> > > > > 
> > > > 
> > > > Is it worth breaking the 80-column limit for this?
> > > 
> > > Deepak,
> > > 
> > > It was much nicer before.
> > > 
> > > It can be awkward to break eg a + operation at the 80 character limit.
> > > But function argument stand by themselves.
> > > 
> > > julia
> > > 
> > 
> > Hi Julia and Ian,
> > I wanted to take advantage of the relaxation of 80 column limit to 100
> > columns and hence proposed combining the lines. Are you saying this is
> > allowed only in certain cases?
> > 
> > Please confirm and I will handle it accordingly.
> 
> Hi Deepak,
> 
> 80 columns is still the preferred limit.  I think the relaxation is mostly
> to avoid the need to split sub-expressions across lines in really ugly ways
> to keep within the 80 columns at the expense of readability.
> 

Thank you Ian. That sounds good. I will just send the corrected patch 1
and will scrap patch 2.

Can I just send a standalone patch as v2 instead of a patch set of
single patch?

Deepak.

> -- 
> -=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
> -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability
  2020-10-19 11:49           ` Deepak R Varma
@ 2020-10-19 12:46             ` Ian Abbott
  0 siblings, 0 replies; 10+ messages in thread
From: Ian Abbott @ 2020-10-19 12:46 UTC (permalink / raw)
  To: Deepak R Varma; +Cc: Julia Lawall, outreachy-kernel, devel, Greg Kroah-Hartman

On 19/10/2020 12:49, Deepak R Varma wrote:
> On Mon, Oct 19, 2020 at 12:34:15PM +0100, Ian Abbott wrote:
>> On 19/10/2020 11:57, Deepak R Varma wrote:
>>> On Mon, Oct 19, 2020 at 12:41:14PM +0200, Julia Lawall wrote:
>>>>
>>>>
>>>> On Mon, 19 Oct 2020, Ian Abbott wrote:
>>>>
>>>>> On 18/10/2020 20:49, Deepak R Varma wrote:
>>>>>> Instructions split on multiple lines can be combined on a single line
>>>>>> for improved readability of the code.
>>>>>>
>>>>>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>>>>>> ---
>>>>>>     .../staging/comedi/drivers/tests/ni_routes_test.c    | 12 ++++--------
>>>>>>     1 file changed, 4 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>>>> b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>>>> index 7db83cf5e4aa..a3b1be623861 100644
>>>>>> --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>>>> +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
>>>>>> @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
>>>>>>     	const struct ni_route_tables *T = &private.routing_tables;
>>>>>>       	init_pci_fake();
>>>>>> -	unittest(!route_register_is_valid(4, O(4), T),
>>>>>> -		 "check for bad source 4-->4\n");
>>>>>> -	unittest(!route_register_is_valid(0, O(1), T),
>>>>>> -		 "find first source\n");
>>>>>> -	unittest(!route_register_is_valid(4, O(6), T),
>>>>>> -		 "find middle source\n");
>>>>>> -	unittest(!route_register_is_valid(9, O(8), T),
>>>>>> -		 "find last source");
>>>>>> +	unittest(!route_register_is_valid(4, O(4), T), "check for bad source
>>>>>> 4-->4\n");
>>>>>> +	unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
>>>>>> +	unittest(!route_register_is_valid(4, O(6), T), "find middle
>>>>>> source\n");
>>>>>> +	unittest(!route_register_is_valid(9, O(8), T), "find last source");
>>>>>>     }
>>>>>>       void test_ni_check_trigger_arg(void)
>>>>>>
>>>>>
>>>>> Is it worth breaking the 80-column limit for this?
>>>>
>>>> Deepak,
>>>>
>>>> It was much nicer before.
>>>>
>>>> It can be awkward to break eg a + operation at the 80 character limit.
>>>> But function argument stand by themselves.
>>>>
>>>> julia
>>>>
>>>
>>> Hi Julia and Ian,
>>> I wanted to take advantage of the relaxation of 80 column limit to 100
>>> columns and hence proposed combining the lines. Are you saying this is
>>> allowed only in certain cases?
>>>
>>> Please confirm and I will handle it accordingly.
>>
>> Hi Deepak,
>>
>> 80 columns is still the preferred limit.  I think the relaxation is mostly
>> to avoid the need to split sub-expressions across lines in really ugly ways
>> to keep within the 80 columns at the expense of readability.
>>
> 
> Thank you Ian. That sounds good. I will just send the corrected patch 1
> and will scrap patch 2.
> 
> Can I just send a standalone patch as v2 instead of a patch set of
> single patch?

That should be fine, but in the notes for "v2" after  the "---" line for 
the now standalone patch, you could mention that patch 2/2 of the series 
has been dropped.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-19 12:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-18 19:48 [PATCH 1/2] staging: comedi: Simplify conditional evaluation Deepak R Varma
2020-10-18 19:49 ` [PATCH 2/2] staging: comedi: combine split lines for improved readability Deepak R Varma
2020-10-19 10:22   ` Ian Abbott
2020-10-19 10:41     ` [Outreachy kernel] " Julia Lawall
2020-10-19 10:57       ` Deepak R Varma
2020-10-19 11:34         ` Ian Abbott
2020-10-19 11:49           ` Deepak R Varma
2020-10-19 12:46             ` Ian Abbott
2020-10-19 10:17 ` [PATCH 1/2] staging: comedi: Simplify conditional evaluation Ian Abbott
2020-10-19 10:51   ` Deepak R Varma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).