Fix " CHECK: Alignment should match open parenthesis "
Reported by checkpath
Signed-off-by: Menna Mahmoud <[email protected]>
---
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
Fix " CHECK: Alignment should match open parenthesis "
Reported by checkpath
Signed-off-by: Menna Mahmoud <[email protected]>
---
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
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 <[email protected]>
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,
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 <[email protected]>
>
> 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,
>
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 <[email protected]>
> ---
> 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
>
>
>
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 <[email protected]>
> ---
> 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
>
>
>
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 <[email protected]>
>> ---
>> 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
>>
>>
>>
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 <[email protected]>
>> ---
>> 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
>>
>>
>>
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 <[email protected]>
> > > ---
> > > 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
> > >
> > >
> > >
>
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 <[email protected]>
>>>> ---
>>>> 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
>>>>
>>>>
>>>>
> >
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 <[email protected]>
>>>>> ---
>>>>> 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
>>>>>
>>>>>
>>>>>
>> >
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
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
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 <[email protected]>
> > > > > > ---
> > > > > > 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
> > > > > >
> > > > > >
> > > > > >
> > > >
>
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.
>
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 <[email protected]>
>>>>>>> ---
>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>>
> >