* [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 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 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-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 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 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: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: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: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).