Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
I am trying to improve my rep first through. Cheers Nick On 14-10-30 12:21 AM, Sudip Mukherjee wrote: > On Thu, Oct 30, 2014 at 5:34 AM, nick wrote: >> I don't mind waiting. I am just honestly trying to improve my rep here and >> actually(hopefully) get a job >> doing this full time. > > great , even if i want to get into a job where i can do this full time > , and hopefully everyone here does, > I dont want to discourage you , but considering your reputation (all > over the world - maybe even outside this world) , that might be > difficult. > >> Cheers Nick >> >> On 14-10-29 04:45 AM, Scott Lovenberg wrote: >>> On Wed, Oct 29, 2014 at 1:48 AM, Sudip Mukherjee >>> wrote: On Wed, Oct 29, 2014 at 11:38 AM, Scott Lovenberg wrote: > On Wed, Oct 29, 2014 at 12:05 AM, Sudip Mukherjee > wrote: >> >> On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: >>> Greg, >>> That's fine, I was wondering how long Greg KH takes to get around to >>> picking this up as he is very busy with >>> other kernel work. >> >> it might take a long long time. i think he is very busy now. I have >> not seen his replies to patches in the kernel list for atleast last 3 >> weeks. >> > [snip] >> >> ___ >> Kernelnewbies mailing list >> Kernelnewbies@kernelnewbies.org >> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies > > > > Nah. Greg KH is a robot. I'm firmly convinced that man doesn't > sleep. If I didn't know better, I'd think he's a Cylon. well said. > On a serious note; realistically, a two week window isn't unheard of > for getting your patches to mainline. So long as you're not trying to mine is three weeks going on now. somehow i have managed to send my patches just at the beginning of the merge window. :( yesterday i saw Greg K-H releasing the stable patches , so i guess now he will be seeing the pending staging patches. >>> >>> It also depends on how many hops you are to the maintainer and how >>> heavy their workload is. Sometimes you can directly submit to them, >>> other times your patches will be passed through three trees before >>> they see mainline. It all depends on what you're working on and who's >>> in your "circle". >>> ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Thu, Oct 30, 2014 at 5:34 AM, nick wrote: > I don't mind waiting. I am just honestly trying to improve my rep here and > actually(hopefully) get a job > doing this full time. great , even if i want to get into a job where i can do this full time , and hopefully everyone here does, I dont want to discourage you , but considering your reputation (all over the world - maybe even outside this world) , that might be difficult. > Cheers Nick > > On 14-10-29 04:45 AM, Scott Lovenberg wrote: >> On Wed, Oct 29, 2014 at 1:48 AM, Sudip Mukherjee >> wrote: >>> On Wed, Oct 29, 2014 at 11:38 AM, Scott Lovenberg >>> wrote: On Wed, Oct 29, 2014 at 12:05 AM, Sudip Mukherjee wrote: > > On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: >> Greg, >> That's fine, I was wondering how long Greg KH takes to get around to >> picking this up as he is very busy with >> other kernel work. > > it might take a long long time. i think he is very busy now. I have > not seen his replies to patches in the kernel list for atleast last 3 > weeks. > [snip] > > ___ > Kernelnewbies mailing list > Kernelnewbies@kernelnewbies.org > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Nah. Greg KH is a robot. I'm firmly convinced that man doesn't sleep. If I didn't know better, I'd think he's a Cylon. >>> well said. >>> On a serious note; realistically, a two week window isn't unheard of for getting your patches to mainline. So long as you're not trying to >>> >>> mine is three weeks going on now. somehow i have managed to send my >>> patches just at the beginning of the merge window. :( >>> yesterday i saw Greg K-H releasing the stable patches , so i guess now >>> he will be seeing the pending staging patches. >> >> It also depends on how many hops you are to the maintainer and how >> heavy their workload is. Sometimes you can directly submit to them, >> other times your patches will be passed through three trees before >> they see mainline. It all depends on what you're working on and who's >> in your "circle". >> ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
I don't mind waiting. I am just honestly trying to improve my rep here and actually(hopefully) get a job doing this full time. Cheers Nick On 14-10-29 04:45 AM, Scott Lovenberg wrote: > On Wed, Oct 29, 2014 at 1:48 AM, Sudip Mukherjee > wrote: >> On Wed, Oct 29, 2014 at 11:38 AM, Scott Lovenberg >> wrote: >>> On Wed, Oct 29, 2014 at 12:05 AM, Sudip Mukherjee >>> wrote: On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: > Greg, > That's fine, I was wondering how long Greg KH takes to get around to > picking this up as he is very busy with > other kernel work. it might take a long long time. i think he is very busy now. I have not seen his replies to patches in the kernel list for atleast last 3 weeks. >>> [snip] ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies >>> >>> >>> >>> Nah. Greg KH is a robot. I'm firmly convinced that man doesn't >>> sleep. If I didn't know better, I'd think he's a Cylon. >> well said. >> >>> On a serious note; realistically, a two week window isn't unheard of >>> for getting your patches to mainline. So long as you're not trying to >> >> mine is three weeks going on now. somehow i have managed to send my >> patches just at the beginning of the merge window. :( >> yesterday i saw Greg K-H releasing the stable patches , so i guess now >> he will be seeing the pending staging patches. > > It also depends on how many hops you are to the maintainer and how > heavy their workload is. Sometimes you can directly submit to them, > other times your patches will be passed through three trees before > they see mainline. It all depends on what you're working on and who's > in your "circle". > ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Wed, Oct 29, 2014 at 1:48 AM, Sudip Mukherjee wrote: > On Wed, Oct 29, 2014 at 11:38 AM, Scott Lovenberg > wrote: >> On Wed, Oct 29, 2014 at 12:05 AM, Sudip Mukherjee >> wrote: >>> >>> On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: >>> > Greg, >>> > That's fine, I was wondering how long Greg KH takes to get around to >>> > picking this up as he is very busy with >>> > other kernel work. >>> >>> it might take a long long time. i think he is very busy now. I have >>> not seen his replies to patches in the kernel list for atleast last 3 >>> weeks. >>> >> [snip] >>> >>> ___ >>> Kernelnewbies mailing list >>> Kernelnewbies@kernelnewbies.org >>> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies >> >> >> >> Nah. Greg KH is a robot. I'm firmly convinced that man doesn't >> sleep. If I didn't know better, I'd think he's a Cylon. > well said. > >> On a serious note; realistically, a two week window isn't unheard of >> for getting your patches to mainline. So long as you're not trying to > > mine is three weeks going on now. somehow i have managed to send my > patches just at the beginning of the merge window. :( > yesterday i saw Greg K-H releasing the stable patches , so i guess now > he will be seeing the pending staging patches. It also depends on how many hops you are to the maintainer and how heavy their workload is. Sometimes you can directly submit to them, other times your patches will be passed through three trees before they see mainline. It all depends on what you're working on and who's in your "circle". -- Peace and Blessings, -Scott. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Wed, Oct 29, 2014 at 11:38 AM, Scott Lovenberg wrote: > On Wed, Oct 29, 2014 at 12:05 AM, Sudip Mukherjee > wrote: >> >> On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: >> > Greg, >> > That's fine, I was wondering how long Greg KH takes to get around to >> > picking this up as he is very busy with >> > other kernel work. >> >> it might take a long long time. i think he is very busy now. I have >> not seen his replies to patches in the kernel list for atleast last 3 >> weeks. >> > [snip] >> >> ___ >> Kernelnewbies mailing list >> Kernelnewbies@kernelnewbies.org >> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies > > > > Nah. Greg KH is a robot. I'm firmly convinced that man doesn't > sleep. If I didn't know better, I'd think he's a Cylon. well said. > On a serious note; realistically, a two week window isn't unheard of > for getting your patches to mainline. So long as you're not trying to mine is three weeks going on now. somehow i have managed to send my patches just at the beginning of the merge window. :( yesterday i saw Greg K-H releasing the stable patches , so i guess now he will be seeing the pending staging patches. > sneak in right as the merge window is closing, you'll make the next > release. In fact, if you submit early enough, you can almost forget > that you have a patch in a kernel when it's released 2 months later. > Nothing like reading the release notes and finding your own patches > that you've forgotten about over your morning coffee. :) > > -- > Peace and Blessings, > -Scott. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Wed, Oct 29, 2014 at 12:05 AM, Sudip Mukherjee wrote: > > On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: > > Greg, > > That's fine, I was wondering how long Greg KH takes to get around to > > picking this up as he is very busy with > > other kernel work. > > it might take a long long time. i think he is very busy now. I have > not seen his replies to patches in the kernel list for atleast last 3 > weeks. > [snip] > > ___ > Kernelnewbies mailing list > Kernelnewbies@kernelnewbies.org > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies Nah. Greg KH is a robot. I'm firmly convinced that man doesn't sleep. If I didn't know better, I'd think he's a Cylon. On a serious note; realistically, a two week window isn't unheard of for getting your patches to mainline. So long as you're not trying to sneak in right as the merge window is closing, you'll make the next release. In fact, if you submit early enough, you can almost forget that you have a patch in a kernel when it's released 2 months later. Nothing like reading the release notes and finding your own patches that you've forgotten about over your morning coffee. :) -- Peace and Blessings, -Scott. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Wed, Oct 29, 2014 at 7:31 AM, nick wrote: > Greg, > That's fine, I was wondering how long Greg KH takes to get around to picking > this up as he is very busy with > other kernel work. it might take a long long time. i think he is very busy now. I have not seen his replies to patches in the kernel list for atleast last 3 weeks. > Cheers Nick > > On 14-10-28 09:58 PM, Jeff Kirsher wrote: >> On Tue, 2014-10-28 at 21:49 -0400, nick wrote: >>> Greg, >>> Not picked up of yet. I would appreciate if this gets forwarded for me as >>> this may help it get picked up. >>> Further more this issues I am were causing were not technical but not >>> listening and that's why I decided >>> to state around and learn how to my patches properly. >>> Cheers Nick >>> >> >> I saw you updated you patch Nick, thanks. I feel I can safely put my >> Reviewed-by: on it. Also, if Greg KH (staging maintainer) has not >> picked up in the next day or so, I will submit your patch to the staging >> tree for you. >> >>> >>> On 14-10-28 05:31 PM, Greg Donald wrote: On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: > I am trying to improve my code as much as possible now, I really am > finally understanding > how terrible my code was before and I hope never again to make patches > that shitty. I don't think your recent progress has gone unnoticed. Any word on your latest patch getting picked up? It looked good to me. You remind me of John Locke from Lost. "Don't tell me what I can't do!" :) >> >> > > ___ > Kernelnewbies mailing list > Kernelnewbies@kernelnewbies.org > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
Sorry Jeff, My fault, mistyped. Cheers Nick On 14-10-28 10:04 PM, Jeff Kirsher wrote: > On Tue, 2014-10-28 at 22:01 -0400, nick wrote: >> Greg, >> That's fine, I was wondering how long Greg KH takes to get around to picking >> this up as he is very busy with >> other kernel work. >> Cheers Nick > > I am Jeff and I was the one that responded... > >> >> On 14-10-28 09:58 PM, Jeff Kirsher wrote: >>> On Tue, 2014-10-28 at 21:49 -0400, nick wrote: Greg, Not picked up of yet. I would appreciate if this gets forwarded for me as this may help it get picked up. Further more this issues I am were causing were not technical but not listening and that's why I decided to state around and learn how to my patches properly. Cheers Nick >>> >>> I saw you updated you patch Nick, thanks. I feel I can safely put my >>> Reviewed-by: on it. Also, if Greg KH (staging maintainer) has not >>> picked up in the next day or so, I will submit your patch to the staging >>> tree for you. >>> On 14-10-28 05:31 PM, Greg Donald wrote: > On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: >> I am trying to improve my code as much as possible now, I really am >> finally understanding >> how terrible my code was before and I hope never again to make patches >> that shitty. > > I don't think your recent progress has gone unnoticed. Any word on > your latest patch getting picked up? It looked good to me. > > You remind me of John Locke from Lost. "Don't tell me what I can't do!" > :) > > >>> >>> > > ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, 2014-10-28 at 22:01 -0400, nick wrote: > Greg, > That's fine, I was wondering how long Greg KH takes to get around to picking > this up as he is very busy with > other kernel work. > Cheers Nick I am Jeff and I was the one that responded... > > On 14-10-28 09:58 PM, Jeff Kirsher wrote: > > On Tue, 2014-10-28 at 21:49 -0400, nick wrote: > >> Greg, > >> Not picked up of yet. I would appreciate if this gets forwarded for me as > >> this may help it get picked up. > >> Further more this issues I am were causing were not technical but not > >> listening and that's why I decided > >> to state around and learn how to my patches properly. > >> Cheers Nick > >> > > > > I saw you updated you patch Nick, thanks. I feel I can safely put my > > Reviewed-by: on it. Also, if Greg KH (staging maintainer) has not > > picked up in the next day or so, I will submit your patch to the staging > > tree for you. > > > >> > >> On 14-10-28 05:31 PM, Greg Donald wrote: > >>> On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: > I am trying to improve my code as much as possible now, I really am > finally understanding > how terrible my code was before and I hope never again to make patches > that shitty. > >>> > >>> I don't think your recent progress has gone unnoticed. Any word on > >>> your latest patch getting picked up? It looked good to me. > >>> > >>> You remind me of John Locke from Lost. "Don't tell me what I can't do!" > >>> :) > >>> > >>> > > > > signature.asc Description: This is a digitally signed message part ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
Greg, That's fine, I was wondering how long Greg KH takes to get around to picking this up as he is very busy with other kernel work. Cheers Nick On 14-10-28 09:58 PM, Jeff Kirsher wrote: > On Tue, 2014-10-28 at 21:49 -0400, nick wrote: >> Greg, >> Not picked up of yet. I would appreciate if this gets forwarded for me as >> this may help it get picked up. >> Further more this issues I am were causing were not technical but not >> listening and that's why I decided >> to state around and learn how to my patches properly. >> Cheers Nick >> > > I saw you updated you patch Nick, thanks. I feel I can safely put my > Reviewed-by: on it. Also, if Greg KH (staging maintainer) has not > picked up in the next day or so, I will submit your patch to the staging > tree for you. > >> >> On 14-10-28 05:31 PM, Greg Donald wrote: >>> On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: I am trying to improve my code as much as possible now, I really am finally understanding how terrible my code was before and I hope never again to make patches that shitty. >>> >>> I don't think your recent progress has gone unnoticed. Any word on >>> your latest patch getting picked up? It looked good to me. >>> >>> You remind me of John Locke from Lost. "Don't tell me what I can't do!" :) >>> >>> > > ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, 2014-10-28 at 21:49 -0400, nick wrote: > Greg, > Not picked up of yet. I would appreciate if this gets forwarded for me as > this may help it get picked up. > Further more this issues I am were causing were not technical but not > listening and that's why I decided > to state around and learn how to my patches properly. > Cheers Nick > I saw you updated you patch Nick, thanks. I feel I can safely put my Reviewed-by: on it. Also, if Greg KH (staging maintainer) has not picked up in the next day or so, I will submit your patch to the staging tree for you. > > On 14-10-28 05:31 PM, Greg Donald wrote: > > On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: > >> I am trying to improve my code as much as possible now, I really am > >> finally understanding > >> how terrible my code was before and I hope never again to make patches > >> that shitty. > > > > I don't think your recent progress has gone unnoticed. Any word on > > your latest patch getting picked up? It looked good to me. > > > > You remind me of John Locke from Lost. "Don't tell me what I can't do!" :) > > > > signature.asc Description: This is a digitally signed message part ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
Greg, Not picked up of yet. I would appreciate if this gets forwarded for me as this may help it get picked up. Further more this issues I am were causing were not technical but not listening and that's why I decided to state around and learn how to my patches properly. Cheers Nick On 14-10-28 05:31 PM, Greg Donald wrote: > On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: >> I am trying to improve my code as much as possible now, I really am finally >> understanding >> how terrible my code was before and I hope never again to make patches that >> shitty. > > I don't think your recent progress has gone unnoticed. Any word on > your latest patch getting picked up? It looked good to me. > > You remind me of John Locke from Lost. "Don't tell me what I can't do!" :) > > ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 03:55:53PM -0500, Greg Donald wrote: > On Tue, Oct 28, 2014 at 2:04 PM, John de la Garza wrote: > > It seems like you assuming the limit is based on terminal size? > > Hmm, I thought it was because of 80 column punch cards and ancient > printers that didn't wrap. > What is the difference? It sounds like you think the 80 col suggesetion is based on hardware limitations, that is my point. > > There is some research in typography that suggests that it is easier > > to read text that is not on long lines. > > Developers don't actually read most of the code seen, over time our > eyes become trained to rapidly scan for patterns. We've invented lots > of tools like cscope, ctags, lxr, and grep to further keep us from > having to actually read much code at all. > Well, some programmers do actually spend a far amount of time reading code. I simply have to disagree with you on this. > > > Newspapers have a large > > sheet of paper but they use short columns. > > Yeah, short columns with chaotic white-space. The text fills the > column width and is both left and right justified. I doubt anyone > does "text-align: justify;" to their code editor. > > Who still reads newspapers? The 2 people still using punch cards? > you are missing my point, I am make an example of how some peoples eyes generally work > > My display is frequently greater than 80 col. I see this as an > > opertunity to have two xterms next to each other, not to have lines > > that are longer. > > Do you hear yourself? "Oh, my eyes, my eyes, I can't be bothered to > read code so far out to the right, it burns, it burns!" yes, this is basically what I am saying. > > Or "I can read two lines of code in two different files from two > different contexts, but I cannot be bothered to read one longer line > from a single file from a single context. I can multitask all day, > but I cannot ever single-task, forget that drama!". now you are assuming things about the way I work Sometimes I tail a log file while working in another window, but this isn't what we are talking about > > All I'm saying is since no one adheres to it religiously, and the fact > that I've seen many times maintainers telling people to just ignore > it, the 80 character checkpatch warning is pointless. Given that it > really comes down to a per-case best effort attempt, just set an 80 > column indicator in your editor and try to stay within it, if you can. you are infering a lot from my original comment. I wasn't saying to stick to it at all costs. I don't split strings because then you may have a hard time grepping for them. If you can't fit your code in 80 columns it is usually a sign you are getting it wrong. I rarely have felt limited by 80 columns. > > Keeping the checkpatch 80 character warning around is just academic > masturbation. submit a patch to change it then, if no one cares then it should get merged, I'm betting people do care ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 2:52 PM, nick wrote: > I am trying to improve my code as much as possible now, I really am finally > understanding > how terrible my code was before and I hope never again to make patches that > shitty. I don't think your recent progress has gone unnoticed. Any word on your latest patch getting picked up? It looked good to me. You remind me of John Locke from Lost. "Don't tell me what I can't do!" :) -- Greg Donald ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 1:20 PM, Mandeep Sandhu wrote: > Just > increase your font size so that it fits nicely in your editor from end to > end! :P I see :) Sounds good but no. Increasing the font size just causes a loss of screen real estate vertically. -- Greg Donald ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 2:04 PM, John de la Garza wrote: > It seems like you assuming the limit is based on terminal size? Hmm, I thought it was because of 80 column punch cards and ancient printers that didn't wrap. > There is some research in typography that suggests that it is easier > to read text that is not on long lines. Developers don't actually read most of the code seen, over time our eyes become trained to rapidly scan for patterns. We've invented lots of tools like cscope, ctags, lxr, and grep to further keep us from having to actually read much code at all. Your editor doesn't have soft wrap? > Newspapers have a large > sheet of paper but they use short columns. Yeah, short columns with chaotic white-space. The text fills the column width and is both left and right justified. I doubt anyone does "text-align: justify;" to their code editor. Who still reads newspapers? The 2 people still using punch cards? > My display is frequently greater than 80 col. I see this as an > opertunity to have two xterms next to each other, not to have lines > that are longer. Do you hear yourself? "Oh, my eyes, my eyes, I can't be bothered to read code so far out to the right, it burns, it burns!" Or "I can read two lines of code in two different files from two different contexts, but I cannot be bothered to read one longer line from a single file from a single context. I can multitask all day, but I cannot ever single-task, forget that drama!". All I'm saying is since no one adheres to it religiously, and the fact that I've seen many times maintainers telling people to just ignore it, the 80 character checkpatch warning is pointless. Given that it really comes down to a per-case best effort attempt, just set an 80 column indicator in your editor and try to stay within it, if you can. Keeping the checkpatch 80 character warning around is just academic masturbation. -- Greg Donald ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
Hey Greg, I am trying to improve my code as much as possible now, I really am finally understanding how terrible my code was before and I hope never again to make patches that shitty. Cheers Nick On 14-10-28 02:06 PM, Greg Donald wrote: > On Tue, Oct 28, 2014 at 12:53 PM, Nick Krause wrote: >> I actually fixed this to improve code readability not for the kernel >> rules for your information. > > I know, good job.. and here's to hoping a maintainer picks up your > patch and applies it to their tree :) > > But I would argue "kernel rules" and "improving code readability" are > one and the same. Otherwise checkpatch and it's optional -f needn't > exist. > > ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 12:39:17PM -0500, Greg Donald wrote: > The WARNING "line over 80 characters" currently accounts for 216K of > the total violations. IMHO checkpatch should just stop complaining > about the 80 char limit since that's the main offender causing new > kernel developers to inadvertently lessen readability with their first > patch. If the 80 char warning should be mostly ignored why have it.. > it's pointless. Increase it to a 21st century value or kill it. > > It seems like you assuming the limit is based on terminal size? There is some research in typography that suggests that it is easier to read text that is not on long lines. Newspapers have a large sheet of paper but they use short columns. My display is frequently greater than 80 col. I see this as an opertunity to have two xterms next to each other, not to have lines that are longer. ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, 28 Oct 2014 12:39:17 -0500, Greg Donald said: > The WARNING "line over 80 characters" currently accounts for 216K of > the total violations. IMHO checkpatch should just stop complaining > about the 80 char limit since that's the main offender causing new On the other hand, there's very good human factors reasons to restrict the length of source lines. As the human eye reads text, it saccades across, and too-long lines result in poor tracking. More than 60-75 characters causes problems. http://en.wikipedia.org/wiki/Eye_movement_in_reading http://baymard.com/blog/line-length-readability The fact that C code is indented doesn't help - it means the left margin isn't as easily predictable as flush-left alignment. pgpxKXdrmRc5h.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
> > > it's pointless. Increase it to a 21st century value or kill it. > > > > > > -- > > Greg Donald > > But, but, but, what about all the kernel developers who are writing kernel > code on VT100s and storing their sources on 80 column punch cards? > Lol, I agree! There's nothing 21st century about long lines! As screen resolutions increase ("Retina 5k" alert!), 1 line for you might, be 2 or more for another. So in this regard, the old school 80 col is just right. Just increase your font size so that it fits nicely in your editor from end to end! :P > > Not everybody can afford these newfangled disk drives, ya know. 8^) > > Jeff Haran > > > ___ > Kernelnewbies mailing list > Kernelnewbies@kernelnewbies.org > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies > ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
RE: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
> -Original Message- > From: kernelnewbies-bounces+jharan=bytemobile@kernelnewbies.org > [mailto:kernelnewbies- > bounces+jharan=bytemobile@kernelnewbies.org] On Behalf Of Greg Donald > Sent: Tuesday, October 28, 2014 10:39 AM > To: Greg Freemyer > Cc: kernelnewbies; Jeff Kirsher; Nicholas Krause > Subject: Re: [PATCH] staging: rtl8723au: Fix brace coding style issues > reported by checkpatch > > On Tue, Oct 28, 2014 at 11:50 AM, Greg Freemyer > wrote: > > Lots of violations > > checkpatch finds are intentionally left in place because correcting > > them makes the code less readable, not more readable. > > Yeah, but there are still hundreds of thousands of checkpatch > violations throughout the kernel that if fixed would actually improve > readability. > > PathErrors Warnings > drivers 200979 361350 > arch98791 142300 > sound 25938 31028 > include 13651 25598 > fs 96353 22483 > net 318519216 > lib 80556578 > tools 11263972 > kernel 656 3203 > security47 1247 > mm 203 1186 > scripts 824 1168 > crypto 11441095 > block 196 656 > Documentation 97 259 > init29 173 > virt18 152 > samples 27 118 > ipc 2 77 > usr 17 20 > firmware11 19 > > The WARNING "line over 80 characters" currently accounts for 216K of > the total violations. IMHO checkpatch should just stop complaining > about the 80 char limit since that's the main offender causing new > kernel developers to inadvertently lessen readability with their first > patch. If the 80 char warning should be mostly ignored why have it.. > it's pointless. Increase it to a 21st century value or kill it. > > > -- > Greg Donald But, but, but, what about all the kernel developers who are writing kernel code on VT100s and storing their sources on 80 column punch cards? Not everybody can afford these newfangled disk drives, ya know. 8^) Jeff Haran ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 12:53 PM, Nick Krause wrote: > I actually fixed this to improve code readability not for the kernel > rules for your information. I know, good job.. and here's to hoping a maintainer picks up your patch and applies it to their tree :) But I would argue "kernel rules" and "improving code readability" are one and the same. Otherwise checkpatch and it's optional -f needn't exist. -- Greg Donald ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 1:39 PM, Greg Donald wrote: > On Tue, Oct 28, 2014 at 11:50 AM, Greg Freemyer > wrote: >> Lots of violations >> checkpatch finds are intentionally left in place because correcting >> them makes the code less readable, not more readable. > > Yeah, but there are still hundreds of thousands of checkpatch > violations throughout the kernel that if fixed would actually improve > readability. > > PathErrors Warnings > drivers 200979 361350 > arch98791 142300 > sound 25938 31028 > include 13651 25598 > fs 96353 22483 > net 318519216 > lib 80556578 > tools 11263972 > kernel 656 3203 > security47 1247 > mm 203 1186 > scripts 824 1168 > crypto 11441095 > block 196 656 > Documentation 97 259 > init29 173 > virt18 152 > samples 27 118 > ipc 2 77 > usr 17 20 > firmware11 19 > > The WARNING "line over 80 characters" currently accounts for 216K of > the total violations. IMHO checkpatch should just stop complaining > about the 80 char limit since that's the main offender causing new > kernel developers to inadvertently lessen readability with their first > patch. If the 80 char warning should be mostly ignored why have it.. > it's pointless. Increase it to a 21st century value or kill it. > > > -- > Greg Donald I actually fixed this to improve code readability not for the kernel rules for your information. Cheers Nick ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 11:50 AM, Greg Freemyer wrote: > Lots of violations > checkpatch finds are intentionally left in place because correcting > them makes the code less readable, not more readable. Yeah, but there are still hundreds of thousands of checkpatch violations throughout the kernel that if fixed would actually improve readability. PathErrors Warnings drivers 200979 361350 arch98791 142300 sound 25938 31028 include 13651 25598 fs 96353 22483 net 318519216 lib 80556578 tools 11263972 kernel 656 3203 security47 1247 mm 203 1186 scripts 824 1168 crypto 11441095 block 196 656 Documentation 97 259 init29 173 virt18 152 samples 27 118 ipc 2 77 usr 17 20 firmware11 19 The WARNING "line over 80 characters" currently accounts for 216K of the total violations. IMHO checkpatch should just stop complaining about the 80 char limit since that's the main offender causing new kernel developers to inadvertently lessen readability with their first patch. If the 80 char warning should be mostly ignored why have it.. it's pointless. Increase it to a 21st century value or kill it. -- Greg Donald ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Tue, Oct 28, 2014 at 4:24 AM, Jeff Kirsher wrote: > On Sun, Oct 26, 2014 at 12:52 PM, Nicholas Krause wrote: >> Fix all opening and closing braces issues reported by checkpatch. >> Signed-off-by: Nicholas Krause >> --- >> drivers/staging/rtl8723au/core/rtw_ap.c | 138 >> ++-- >> 1 file changed, 43 insertions(+), 95 deletions(-) > > Good job Nick, don't let the compliment go to your head now. I am > going to be very nitpicky because of past patch problems, so with that > said, the patch description could be improve a little. I know that > with simple changes like this, it is tough to not repeat yourself > (i.e. title is the same as the description). Currently the title and > patch description are ok, but personally I prefer that the patch > description be more verbose and not an copy of the patch title. So in > this case, a patch description of: > > "Fix the curley braces that do not reside on the same line because > this does not follow the kernel coding style and causes checkpatch.pl > warnings." That's actually important. Using checkpatch to identify potential problems is smart, but you fix them because the violate the kernel coding standard and there is no overriding reason to allow the violation. Lots of violations checkpatch finds are intentionally left in place because correcting them makes the code less readable, not more readable. Greg -- Greg Freemyer ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: [PATCH] staging: rtl8723au: Fix brace coding style issues reported by checkpatch
On Sun, Oct 26, 2014 at 12:52 PM, Nicholas Krause wrote: > Fix all opening and closing braces issues reported by checkpatch. > Signed-off-by: Nicholas Krause > --- > drivers/staging/rtl8723au/core/rtw_ap.c | 138 > ++-- > 1 file changed, 43 insertions(+), 95 deletions(-) Good job Nick, don't let the compliment go to your head now. I am going to be very nitpicky because of past patch problems, so with that said, the patch description could be improve a little. I know that with simple changes like this, it is tough to not repeat yourself (i.e. title is the same as the description). Currently the title and patch description are ok, but personally I prefer that the patch description be more verbose and not an copy of the patch title. So in this case, a patch description of: "Fix the curley braces that do not reside on the same line because this does not follow the kernel coding style and causes checkpatch.pl warnings." I am not saying you have to change it, but something to keep in mind. > > diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c > b/drivers/staging/rtl8723au/core/rtw_ap.c > index 75ae249..3301a2c 100644 > --- a/drivers/staging/rtl8723au/core/rtw_ap.c > +++ b/drivers/staging/rtl8723au/core/rtw_ap.c > @@ -231,12 +231,10 @@ void expire_timeout_chk23a(struct rtw_adapter > *padapter) > psta->expire_to--; > } > > - if (psta->expire_to <= 0) > - { > + if (psta->expire_to <= 0) { > struct mlme_ext_priv *pmlmeext = > &padapter->mlmeextpriv; > > - if (padapter->registrypriv.wifi_spec == 1) > - { > + if (padapter->registrypriv.wifi_spec == 1) { > psta->expire_to = pstapriv->expire_to; > continue; > } > @@ -308,15 +306,12 @@ void expire_timeout_chk23a(struct rtw_adapter > *padapter) > ret = issue_nulldata23a(padapter, psta->hwaddr, 0, 3, > 50); > > psta->keep_alive_trycnt++; > - if (ret == _SUCCESS) > - { > + if (ret == _SUCCESS) { > DBG_8723A("asoc check, sta(" MAC_FMT ") is alive\n", > MAC_ARG(psta->hwaddr)); > psta->expire_to = pstapriv->expire_to; > psta->keep_alive_trycnt = 0; > continue; > - } > - else if (psta->keep_alive_trycnt <= 3) > - { > + } else if (psta->keep_alive_trycnt <= 3) { > DBG_8723A("ack check for asoc expire, > keep_alive_trycnt =%d\n", psta->keep_alive_trycnt); > psta->expire_to = 1; > continue; > @@ -363,8 +358,7 @@ void add_RATid23a(struct rtw_adapter *padapter, struct > sta_info *psta, u8 rssi_l > return; > > /* b/g mode ra_bitmap */ > - for (i = 0; i < sizeof(psta->bssrateset); i++) > - { > + for (i = 0; i < sizeof(psta->bssrateset); i++) { > if (psta->bssrateset[i]) > tx_ra_bitmap |= > rtw_get_bit_value_from_ieee_value23a(psta->bssrateset[i]&0x7f); > } > @@ -406,8 +400,7 @@ void add_RATid23a(struct rtw_adapter *padapter, struct > sta_info *psta, u8 rssi_l > raid = networktype_to_raid23a(sta_band); > init_rate = get_highest_rate_idx23a(tx_ra_bitmap&0x0fff)&0x3f; > > - if (psta->aid < NUM_STA) > - { > + if (psta->aid < NUM_STA) { > u8 arg = 0; > > arg = psta->mac_id&0x1f; > @@ -436,9 +429,7 @@ void add_RATid23a(struct rtw_adapter *padapter, struct > sta_info *psta, u8 rssi_l > psta->raid = raid; > psta->init_rate = init_rate; > > - } > - else > - { > + } else { > DBG_8723A("station aid %d exceed the max number\n", > psta->aid); > } > } > @@ -453,8 +444,7 @@ static void update_bmc_sta(struct rtw_adapter *padapter) > struct wlan_bssid_ex *pcur_network = &pmlmepriv->cur_network.network; > struct sta_info *psta = rtw_get_bcmc_stainfo23a(padapter); > > - if (psta) > - { > + if (psta) { > psta->aid = 0;/* default set to 0 */ > psta->mac_id = psta->aid + 1; > > @@ -474,8 +464,7 @@ static void update_bmc_sta(struct rtw_adapter *padapter) > psta->bssratelen = supportRateNum; > > /* b/g mode ra_bitmap */ > - for (i = 0; i < supportRateNum; i++) > - { > + for (i = 0; i < supportRateNum; i++) { > if (psta->bssrateset[i]) > tx_ra_bitmap |= > rtw_get_bit_value_from_ieee_value23a(psta->bssrateset[i]&0x7f); > } > @@ -522,9 +511,7 @@