linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
@ 2023-03-10 15:51 Menna Mahmoud
  2023-03-10 15:51 ` [PATCH 2/2] " Menna Mahmoud
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-10 15:51 UTC (permalink / raw)
  To: outreachy
  Cc: vireshk, johan, elder, gregkh, greybus-dev, linux-staging,
	linux-kernel, eng.mennamahmoud.mm

Fix " CHECK: Alignment should match open parenthesis "
Reported by checkpath

Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
---
 drivers/staging/greybus/fw-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
index 57bebf24636b..f562cb12d5ad 100644
--- a/drivers/staging/greybus/fw-core.c
+++ b/drivers/staging/greybus/fw-core.c
@@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
 			}
 
 			connection = gb_connection_create(bundle, cport_id,
-						gb_fw_mgmt_request_handler);
+							  gb_fw_mgmt_request_handler);
 			if (IS_ERR(connection)) {
 				ret = PTR_ERR(connection);
 				dev_err(&bundle->dev,
-- 
2.34.1


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

* [PATCH 2/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-10 15:51 [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis Menna Mahmoud
@ 2023-03-10 15:51 ` Menna Mahmoud
  2023-03-11  8:57   ` Julia Lawall
  2023-03-10 16:16 ` [PATCH 1/2] " Alex Elder
  2023-03-11  8:59 ` Julia Lawall
  2 siblings, 1 reply; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-10 15:51 UTC (permalink / raw)
  To: outreachy
  Cc: vireshk, johan, elder, gregkh, greybus-dev, linux-staging,
	linux-kernel, eng.mennamahmoud.mm

Fix " CHECK: Alignment should match open parenthesis "
Reported by checkpath

Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
---
 drivers/staging/greybus/fw-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
index f562cb12d5ad..0fb15a60412f 100644
--- a/drivers/staging/greybus/fw-core.c
+++ b/drivers/staging/greybus/fw-core.c
@@ -110,7 +110,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
 			}
 
 			connection = gb_connection_create(bundle, cport_id,
-						gb_fw_download_request_handler);
+							  gb_fw_download_request_handler);
 			if (IS_ERR(connection)) {
 				dev_err(&bundle->dev, "failed to create download connection (%ld)\n",
 					PTR_ERR(connection));
-- 
2.34.1


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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-10 15:51 [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis Menna Mahmoud
  2023-03-10 15:51 ` [PATCH 2/2] " Menna Mahmoud
@ 2023-03-10 16:16 ` Alex Elder
  2023-03-10 19:29   ` Menna Mahmoud
  2023-03-11  8:59 ` Julia Lawall
  2 siblings, 1 reply; 16+ messages in thread
From: Alex Elder @ 2023-03-10 16:16 UTC (permalink / raw)
  To: Menna Mahmoud, outreachy
  Cc: vireshk, johan, elder, gregkh, greybus-dev, linux-staging, linux-kernel

On 3/10/23 9:51 AM, Menna Mahmoud wrote:
> Fix " CHECK: Alignment should match open parenthesis "
> Reported by checkpath
> 
> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>

Is this (and your second patch) the only place(s) where
this issue gets reported?

I think this type of alignment is not a major problem,
and alignment isn't done this way in general in this
driver, it's probably OK to keep it that way.

					-Alex

> ---
>   drivers/staging/greybus/fw-core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
> index 57bebf24636b..f562cb12d5ad 100644
> --- a/drivers/staging/greybus/fw-core.c
> +++ b/drivers/staging/greybus/fw-core.c
> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>   			}
>   
>   			connection = gb_connection_create(bundle, cport_id,
> -						gb_fw_mgmt_request_handler);
> +							  gb_fw_mgmt_request_handler);
>   			if (IS_ERR(connection)) {
>   				ret = PTR_ERR(connection);
>   				dev_err(&bundle->dev,


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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-10 16:16 ` [PATCH 1/2] " Alex Elder
@ 2023-03-10 19:29   ` Menna Mahmoud
  0 siblings, 0 replies; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-10 19:29 UTC (permalink / raw)
  To: Alex Elder, outreachy
  Cc: vireshk, johan, elder, gregkh, greybus-dev, linux-staging, linux-kernel


On ١٠‏/٣‏/٢٠٢٣ ١٨:١٦, Alex Elder wrote:
> On 3/10/23 9:51 AM, Menna Mahmoud wrote:
>> Fix " CHECK: Alignment should match open parenthesis "
>> Reported by checkpath
>>
>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>
> Is this (and your second patch) the only place(s) where
> this issue gets reported?

> yes, after fixed them i got 0 check.

> I think this type of alignment is not a major problem,
> and alignment isn't done this way in general in this
> driver, it's probably OK to keep it that way.

Okay, thanks.

  -Menna

>
>                     -Alex
>
>> ---
>>   drivers/staging/greybus/fw-core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/greybus/fw-core.c 
>> b/drivers/staging/greybus/fw-core.c
>> index 57bebf24636b..f562cb12d5ad 100644
>> --- a/drivers/staging/greybus/fw-core.c
>> +++ b/drivers/staging/greybus/fw-core.c
>> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>>               }
>>                 connection = gb_connection_create(bundle, cport_id,
>> -                        gb_fw_mgmt_request_handler);
>> +                              gb_fw_mgmt_request_handler);
>>               if (IS_ERR(connection)) {
>>                   ret = PTR_ERR(connection);
>>                   dev_err(&bundle->dev,
>

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

* Re: [PATCH 2/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-10 15:51 ` [PATCH 2/2] " Menna Mahmoud
@ 2023-03-11  8:57   ` Julia Lawall
  2023-03-11 12:50     ` Menna Mahmoud
  0 siblings, 1 reply; 16+ messages in thread
From: Julia Lawall @ 2023-03-11  8:57 UTC (permalink / raw)
  To: Menna Mahmoud
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel



On Fri, 10 Mar 2023, Menna Mahmoud wrote:

> Fix " CHECK: Alignment should match open parenthesis "
> Reported by checkpath

The log message could be better, to explain what you have done and why.
The word "fix" doesn't express any of that, and should be avoided if
possible.

julia

>
> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> ---
>  drivers/staging/greybus/fw-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
> index f562cb12d5ad..0fb15a60412f 100644
> --- a/drivers/staging/greybus/fw-core.c
> +++ b/drivers/staging/greybus/fw-core.c
> @@ -110,7 +110,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>  			}
>
>  			connection = gb_connection_create(bundle, cport_id,
> -						gb_fw_download_request_handler);
> +							  gb_fw_download_request_handler);
>  			if (IS_ERR(connection)) {
>  				dev_err(&bundle->dev, "failed to create download connection (%ld)\n",
>  					PTR_ERR(connection));
> --
> 2.34.1
>
>
>

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-10 15:51 [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis Menna Mahmoud
  2023-03-10 15:51 ` [PATCH 2/2] " Menna Mahmoud
  2023-03-10 16:16 ` [PATCH 1/2] " Alex Elder
@ 2023-03-11  8:59 ` Julia Lawall
  2023-03-11 12:52   ` Menna Mahmoud
  2 siblings, 1 reply; 16+ messages in thread
From: Julia Lawall @ 2023-03-11  8:59 UTC (permalink / raw)
  To: Menna Mahmoud
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel



On Fri, 10 Mar 2023, Menna Mahmoud wrote:

> Fix " CHECK: Alignment should match open parenthesis "
> Reported by checkpath

See the message in the other mail about the log message.

Also, you should not have two patches with the same subject.  Here, the
changes are on the same file and are essentially the same, even involving
the same function call.  So they can be together in one patch.

julia

>
> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> ---
>  drivers/staging/greybus/fw-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
> index 57bebf24636b..f562cb12d5ad 100644
> --- a/drivers/staging/greybus/fw-core.c
> +++ b/drivers/staging/greybus/fw-core.c
> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>  			}
>
>  			connection = gb_connection_create(bundle, cport_id,
> -						gb_fw_mgmt_request_handler);
> +							  gb_fw_mgmt_request_handler);
>  			if (IS_ERR(connection)) {
>  				ret = PTR_ERR(connection);
>  				dev_err(&bundle->dev,
> --
> 2.34.1
>
>
>

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

* Re: [PATCH 2/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11  8:57   ` Julia Lawall
@ 2023-03-11 12:50     ` Menna Mahmoud
  0 siblings, 0 replies; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-11 12:50 UTC (permalink / raw)
  To: Julia Lawall
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel


On ١١‏/٣‏/٢٠٢٣ ١٠:٥٧, Julia Lawall wrote:
>
> On Fri, 10 Mar 2023, Menna Mahmoud wrote:
>
>> Fix " CHECK: Alignment should match open parenthesis "
>> Reported by checkpath
> The log message could be better, to explain what you have done and why.
> The word "fix" doesn't express any of that, and should be avoided if
> possible.
>
> julia
got it, thank you
>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>> ---
>>   drivers/staging/greybus/fw-core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
>> index f562cb12d5ad..0fb15a60412f 100644
>> --- a/drivers/staging/greybus/fw-core.c
>> +++ b/drivers/staging/greybus/fw-core.c
>> @@ -110,7 +110,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>>   			}
>>
>>   			connection = gb_connection_create(bundle, cport_id,
>> -						gb_fw_download_request_handler);
>> +							  gb_fw_download_request_handler);
>>   			if (IS_ERR(connection)) {
>>   				dev_err(&bundle->dev, "failed to create download connection (%ld)\n",
>>   					PTR_ERR(connection));
>> --
>> 2.34.1
>>
>>
>>

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11  8:59 ` Julia Lawall
@ 2023-03-11 12:52   ` Menna Mahmoud
  2023-03-11 12:55     ` Julia Lawall
  0 siblings, 1 reply; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-11 12:52 UTC (permalink / raw)
  To: Julia Lawall
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel


On ١١‏/٣‏/٢٠٢٣ ١٠:٥٩, Julia Lawall wrote:
>
> On Fri, 10 Mar 2023, Menna Mahmoud wrote:
>
>> Fix " CHECK: Alignment should match open parenthesis "
>> Reported by checkpath
> See the message in the other mail about the log message.
>
> Also, you should not have two patches with the same subject.  Here, the
> changes are on the same file and are essentially the same, even involving
> the same function call.  So they can be together in one patch.
>
> julia
okay, I will. appreciate your feedback. thanks.
>
>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>> ---
>>   drivers/staging/greybus/fw-core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/greybus/fw-core.c b/drivers/staging/greybus/fw-core.c
>> index 57bebf24636b..f562cb12d5ad 100644
>> --- a/drivers/staging/greybus/fw-core.c
>> +++ b/drivers/staging/greybus/fw-core.c
>> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>>   			}
>>
>>   			connection = gb_connection_create(bundle, cport_id,
>> -						gb_fw_mgmt_request_handler);
>> +							  gb_fw_mgmt_request_handler);
>>   			if (IS_ERR(connection)) {
>>   				ret = PTR_ERR(connection);
>>   				dev_err(&bundle->dev,
>> --
>> 2.34.1
>>
>>
>>

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 12:52   ` Menna Mahmoud
@ 2023-03-11 12:55     ` Julia Lawall
  2023-03-11 12:57       ` Menna Mahmoud
  0 siblings, 1 reply; 16+ messages in thread
From: Julia Lawall @ 2023-03-11 12:55 UTC (permalink / raw)
  To: Menna Mahmoud
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]



On Sat, 11 Mar 2023, Menna Mahmoud wrote:

>
> On ١١/٣/٢٠٢٣ ١٠:٥٩, Julia Lawall wrote:
> >
> > On Fri, 10 Mar 2023, Menna Mahmoud wrote:
> >
> > > Fix " CHECK: Alignment should match open parenthesis "
> > > Reported by checkpath
> > See the message in the other mail about the log message.
> >
> > Also, you should not have two patches with the same subject.  Here, the
> > changes are on the same file and are essentially the same, even involving
> > the same function call.  So they can be together in one patch.
> >
> > julia
> okay, I will. appreciate your feedback. thanks.

Please put some blank lines around your response, so it is easier to find.

thanks,
julia

> >
> > > Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> > > ---
> > >   drivers/staging/greybus/fw-core.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/greybus/fw-core.c
> > > b/drivers/staging/greybus/fw-core.c
> > > index 57bebf24636b..f562cb12d5ad 100644
> > > --- a/drivers/staging/greybus/fw-core.c
> > > +++ b/drivers/staging/greybus/fw-core.c
> > > @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
> > >   			}
> > >
> > >   			connection = gb_connection_create(bundle, cport_id,
> > > -						gb_fw_mgmt_request_handler);
> > > +
> > > gb_fw_mgmt_request_handler);
> > >   			if (IS_ERR(connection)) {
> > >   				ret = PTR_ERR(connection);
> > >   				dev_err(&bundle->dev,
> > > --
> > > 2.34.1
> > >
> > >
> > >
>

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 12:55     ` Julia Lawall
@ 2023-03-11 12:57       ` Menna Mahmoud
  2023-03-11 14:06         ` Menna Mahmoud
  0 siblings, 1 reply; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-11 12:57 UTC (permalink / raw)
  To: Julia Lawall
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel


On ١١‏/٣‏/٢٠٢٣ ١٤:٥٥, Julia Lawall wrote:
>
> On Sat, 11 Mar 2023, Menna Mahmoud wrote:
>
>> On ١١/٣/٢٠٢٣ ١٠:٥٩, Julia Lawall wrote:
>>> On Fri, 10 Mar 2023, Menna Mahmoud wrote:
>>>
>>>> Fix " CHECK: Alignment should match open parenthesis "
>>>> Reported by checkpath
>>> See the message in the other mail about the log message.
>>>
>>> Also, you should not have two patches with the same subject.  Here, the
>>> changes are on the same file and are essentially the same, even involving
>>> the same function call.  So they can be together in one patch.
>>>
>>> julia
>> okay, I will. appreciate your feedback. thanks.
> Please put some blank lines around your response, so it is easier to find.
>
> thanks,
> julia


Okay, I will.

thanks,

Menna


>>>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>>>> ---
>>>>    drivers/staging/greybus/fw-core.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/greybus/fw-core.c
>>>> b/drivers/staging/greybus/fw-core.c
>>>> index 57bebf24636b..f562cb12d5ad 100644
>>>> --- a/drivers/staging/greybus/fw-core.c
>>>> +++ b/drivers/staging/greybus/fw-core.c
>>>> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle *bundle,
>>>>    			}
>>>>
>>>>    			connection = gb_connection_create(bundle, cport_id,
>>>> -						gb_fw_mgmt_request_handler);
>>>> +
>>>> gb_fw_mgmt_request_handler);
>>>>    			if (IS_ERR(connection)) {
>>>>    				ret = PTR_ERR(connection);
>>>>    				dev_err(&bundle->dev,
>>>> --
>>>> 2.34.1
>>>>
>>>>
>>>>
> >

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 12:57       ` Menna Mahmoud
@ 2023-03-11 14:06         ` Menna Mahmoud
  2023-03-11 14:23           ` Dan Carpenter
  2023-03-11 14:38           ` Julia Lawall
  0 siblings, 2 replies; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-11 14:06 UTC (permalink / raw)
  To: Julia Lawall
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel


On ١١‏/٣‏/٢٠٢٣ ١٤:٥٧, Menna Mahmoud wrote:
>
> On ١١‏/٣‏/٢٠٢٣ ١٤:٥٥, Julia Lawall wrote:
>>
>> On Sat, 11 Mar 2023, Menna Mahmoud wrote:
>>
>>> On ١١/٣/٢٠٢٣ ١٠:٥٩, Julia Lawall wrote:
>>>> On Fri, 10 Mar 2023, Menna Mahmoud wrote:
>>>>
>>>>> Fix " CHECK: Alignment should match open parenthesis "
>>>>> Reported by checkpath
>>>> See the message in the other mail about the log message.
>>>>
>>>> Also, you should not have two patches with the same subject.  Here, 
>>>> the
>>>> changes are on the same file and are essentially the same, even 
>>>> involving
>>>> the same function call.  So they can be together in one patch.
>>>>
>>>> julia
>>> okay, I will. appreciate your feedback. thanks.
>> Please put some blank lines around your response, so it is easier to 
>> find.
>>
>> thanks,
>> julia
>
>
> Okay, I will.
>
> thanks,
>
> Menna



Hi Julia,

according to Alex feedback

" I think this type of alignment is not a major problem,
and alignment isn't done this way in general in this
driver, it's probably OK to keep it that way. - Alex "


,I won't resubmit these patches, right?


  -Menna


>
>
>>>>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>>>>> ---
>>>>>    drivers/staging/greybus/fw-core.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/staging/greybus/fw-core.c
>>>>> b/drivers/staging/greybus/fw-core.c
>>>>> index 57bebf24636b..f562cb12d5ad 100644
>>>>> --- a/drivers/staging/greybus/fw-core.c
>>>>> +++ b/drivers/staging/greybus/fw-core.c
>>>>> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle 
>>>>> *bundle,
>>>>>                }
>>>>>
>>>>>                connection = gb_connection_create(bundle, cport_id,
>>>>> -                        gb_fw_mgmt_request_handler);
>>>>> +
>>>>> gb_fw_mgmt_request_handler);
>>>>>                if (IS_ERR(connection)) {
>>>>>                    ret = PTR_ERR(connection);
>>>>>                    dev_err(&bundle->dev,
>>>>> -- 
>>>>> 2.34.1
>>>>>
>>>>>
>>>>>
>> >

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 14:06         ` Menna Mahmoud
@ 2023-03-11 14:23           ` Dan Carpenter
  2023-03-11 14:26             ` Dan Carpenter
  2023-03-11 14:57             ` Menna Mahmoud
  2023-03-11 14:38           ` Julia Lawall
  1 sibling, 2 replies; 16+ messages in thread
From: Dan Carpenter @ 2023-03-11 14:23 UTC (permalink / raw)
  To: Menna Mahmoud
  Cc: Julia Lawall, outreachy, vireshk, johan, elder, gregkh,
	greybus-dev, linux-staging, linux-kernel

On Sat, Mar 11, 2023 at 04:06:51PM +0200, Menna Mahmoud wrote:
> according to Alex feedback
> 
> " I think this type of alignment is not a major problem,
> and alignment isn't done this way in general in this
> driver, it's probably OK to keep it that way. - Alex "
> 
> 
> ,I won't resubmit these patches, right?

Yeah.  Find something else to fix.

I feel like in outreachy and similar that people send a first patch and
then they get a bunch of different feedback.  And it's like checkpatch
is complaining and it's staging code so probably the code is actually
ugly in a way.  But often it's better to abandon the project instead of
getting obsessed with it.  Zoom out a bit.  Find something else where
it's obvious how to improve the code from a readability perspective.

People are giving you feedback to help you and not because they are
about that particular line of code.  They won't care if you work on
something else instead.

regards,
dan carpenter


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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 14:23           ` Dan Carpenter
@ 2023-03-11 14:26             ` Dan Carpenter
  2023-03-11 14:57             ` Menna Mahmoud
  1 sibling, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2023-03-11 14:26 UTC (permalink / raw)
  To: Menna Mahmoud
  Cc: Julia Lawall, outreachy, vireshk, johan, elder, gregkh,
	greybus-dev, linux-staging, linux-kernel

On Sat, Mar 11, 2023 at 05:23:11PM +0300, Dan Carpenter wrote:
> People are giving you feedback to help you and not because they are
                                                                  ^^^
I meant "care" not "are".

> about that particular line of code.

regards,
dan carpenter


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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 14:06         ` Menna Mahmoud
  2023-03-11 14:23           ` Dan Carpenter
@ 2023-03-11 14:38           ` Julia Lawall
  2023-03-11 14:58             ` Menna Mahmoud
  1 sibling, 1 reply; 16+ messages in thread
From: Julia Lawall @ 2023-03-11 14:38 UTC (permalink / raw)
  To: Menna Mahmoud
  Cc: Julia Lawall, outreachy, vireshk, johan, elder, gregkh,
	greybus-dev, linux-staging, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]



On Sat, 11 Mar 2023, Menna Mahmoud wrote:

>
> On ١١/٣/٢٠٢٣ ١٤:٥٧, Menna Mahmoud wrote:
> >
> > On ١١/٣/٢٠٢٣ ١٤:٥٥, Julia Lawall wrote:
> > >
> > > On Sat, 11 Mar 2023, Menna Mahmoud wrote:
> > >
> > > > On ١١/٣/٢٠٢٣ ١٠:٥٩, Julia Lawall wrote:
> > > > > On Fri, 10 Mar 2023, Menna Mahmoud wrote:
> > > > >
> > > > > > Fix " CHECK: Alignment should match open parenthesis "
> > > > > > Reported by checkpath
> > > > > See the message in the other mail about the log message.
> > > > >
> > > > > Also, you should not have two patches with the same subject.  Here,
> > > > > the
> > > > > changes are on the same file and are essentially the same, even
> > > > > involving
> > > > > the same function call.  So they can be together in one patch.
> > > > >
> > > > > julia
> > > > okay, I will. appreciate your feedback. thanks.
> > > Please put some blank lines around your response, so it is easier to find.
> > >
> > > thanks,
> > > julia
> >
> >
> > Okay, I will.
> >
> > thanks,
> >
> > Menna
>
>
>
> Hi Julia,
>
> according to Alex feedback
>
> " I think this type of alignment is not a major problem,
> and alignment isn't done this way in general in this
> driver, it's probably OK to keep it that way. - Alex "
>
>
> ,I won't resubmit these patches, right?

The existing code indeed looks better to me.  So you can skip this issue.

julia


>
>
>  -Menna
>
>
> >
> >
> > > > > > Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> > > > > > ---
> > > > > >    drivers/staging/greybus/fw-core.c | 2 +-
> > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/greybus/fw-core.c
> > > > > > b/drivers/staging/greybus/fw-core.c
> > > > > > index 57bebf24636b..f562cb12d5ad 100644
> > > > > > --- a/drivers/staging/greybus/fw-core.c
> > > > > > +++ b/drivers/staging/greybus/fw-core.c
> > > > > > @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle
> > > > > > *bundle,
> > > > > >                }
> > > > > >
> > > > > >                connection = gb_connection_create(bundle, cport_id,
> > > > > > -                        gb_fw_mgmt_request_handler);
> > > > > > +
> > > > > > gb_fw_mgmt_request_handler);
> > > > > >                if (IS_ERR(connection)) {
> > > > > >                    ret = PTR_ERR(connection);
> > > > > >                    dev_err(&bundle->dev,
> > > > > > --
> > > > > > 2.34.1
> > > > > >
> > > > > >
> > > > > >
> > > >
>

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 14:23           ` Dan Carpenter
  2023-03-11 14:26             ` Dan Carpenter
@ 2023-03-11 14:57             ` Menna Mahmoud
  1 sibling, 0 replies; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-11 14:57 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, outreachy, vireshk, johan, elder, gregkh,
	greybus-dev, linux-staging, linux-kernel


On ١١‏/٣‏/٢٠٢٣ ١٦:٢٣, Dan Carpenter wrote:
> On Sat, Mar 11, 2023 at 04:06:51PM +0200, Menna Mahmoud wrote:
>> according to Alex feedback
>>
>> " I think this type of alignment is not a major problem,
>> and alignment isn't done this way in general in this
>> driver, it's probably OK to keep it that way. - Alex "
>>
>>
>> ,I won't resubmit these patches, right?
> Yeah.  Find something else to fix.
>
> I feel like in outreachy and similar that people send a first patch and
> then they get a bunch of different feedback.  And it's like checkpatch
> is complaining and it's staging code so probably the code is actually
> ugly in a way.  But often it's better to abandon the project instead of
> getting obsessed with it.  Zoom out a bit.  Find something else where
> it's obvious how to improve the code from a readability perspective.
>
> People are giving you feedback to help you and not because they are
> about that particular line of code.  They won't care if you work on
> something else instead.
>
> regards,
> dan carpenter


Got it, thanks Dan.


>

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

* Re: [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis
  2023-03-11 14:38           ` Julia Lawall
@ 2023-03-11 14:58             ` Menna Mahmoud
  0 siblings, 0 replies; 16+ messages in thread
From: Menna Mahmoud @ 2023-03-11 14:58 UTC (permalink / raw)
  To: Julia Lawall
  Cc: outreachy, vireshk, johan, elder, gregkh, greybus-dev,
	linux-staging, linux-kernel


On ١١‏/٣‏/٢٠٢٣ ١٦:٣٨, Julia Lawall wrote:
>
> On Sat, 11 Mar 2023, Menna Mahmoud wrote:
>
>> On ١١/٣/٢٠٢٣ ١٤:٥٧, Menna Mahmoud wrote:
>>> On ١١/٣/٢٠٢٣ ١٤:٥٥, Julia Lawall wrote:
>>>> On Sat, 11 Mar 2023, Menna Mahmoud wrote:
>>>>
>>>>> On ١١/٣/٢٠٢٣ ١٠:٥٩, Julia Lawall wrote:
>>>>>> On Fri, 10 Mar 2023, Menna Mahmoud wrote:
>>>>>>
>>>>>>> Fix " CHECK: Alignment should match open parenthesis "
>>>>>>> Reported by checkpath
>>>>>> See the message in the other mail about the log message.
>>>>>>
>>>>>> Also, you should not have two patches with the same subject.  Here,
>>>>>> the
>>>>>> changes are on the same file and are essentially the same, even
>>>>>> involving
>>>>>> the same function call.  So they can be together in one patch.
>>>>>>
>>>>>> julia
>>>>> okay, I will. appreciate your feedback. thanks.
>>>> Please put some blank lines around your response, so it is easier to find.
>>>>
>>>> thanks,
>>>> julia
>>>
>>> Okay, I will.
>>>
>>> thanks,
>>>
>>> Menna
>>
>>
>> Hi Julia,
>>
>> according to Alex feedback
>>
>> " I think this type of alignment is not a major problem,
>> and alignment isn't done this way in general in this
>> driver, it's probably OK to keep it that way. - Alex "
>>
>>
>> ,I won't resubmit these patches, right?
> The existing code indeed looks better to me.  So you can skip this issue.
>
> julia


Okay, thanks Julia.


>
>>
>>   -Menna
>>
>>
>>>
>>>>>>> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
>>>>>>> ---
>>>>>>>     drivers/staging/greybus/fw-core.c | 2 +-
>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/staging/greybus/fw-core.c
>>>>>>> b/drivers/staging/greybus/fw-core.c
>>>>>>> index 57bebf24636b..f562cb12d5ad 100644
>>>>>>> --- a/drivers/staging/greybus/fw-core.c
>>>>>>> +++ b/drivers/staging/greybus/fw-core.c
>>>>>>> @@ -89,7 +89,7 @@ static int gb_fw_core_probe(struct gb_bundle
>>>>>>> *bundle,
>>>>>>>                 }
>>>>>>>
>>>>>>>                 connection = gb_connection_create(bundle, cport_id,
>>>>>>> -                        gb_fw_mgmt_request_handler);
>>>>>>> +
>>>>>>> gb_fw_mgmt_request_handler);
>>>>>>>                 if (IS_ERR(connection)) {
>>>>>>>                     ret = PTR_ERR(connection);
>>>>>>>                     dev_err(&bundle->dev,
>>>>>>> --
>>>>>>> 2.34.1
>>>>>>>
>>>>>>>
>>>>>>>
> >

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

end of thread, other threads:[~2023-03-11 14:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-10 15:51 [PATCH 1/2] staging: greybus: Fix Alignment with parenthesis Menna Mahmoud
2023-03-10 15:51 ` [PATCH 2/2] " Menna Mahmoud
2023-03-11  8:57   ` Julia Lawall
2023-03-11 12:50     ` Menna Mahmoud
2023-03-10 16:16 ` [PATCH 1/2] " Alex Elder
2023-03-10 19:29   ` Menna Mahmoud
2023-03-11  8:59 ` Julia Lawall
2023-03-11 12:52   ` Menna Mahmoud
2023-03-11 12:55     ` Julia Lawall
2023-03-11 12:57       ` Menna Mahmoud
2023-03-11 14:06         ` Menna Mahmoud
2023-03-11 14:23           ` Dan Carpenter
2023-03-11 14:26             ` Dan Carpenter
2023-03-11 14:57             ` Menna Mahmoud
2023-03-11 14:38           ` Julia Lawall
2023-03-11 14:58             ` Menna Mahmoud

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).