From: Chen Gang <chengang@emindsoft.com.cn> To: Mel Gorman <mgorman@techsingularity.net> Cc: "Theodore Ts'o" <tytso@mit.edu>, Jianyu Zhan <nasa4836@gmail.com>, trivial@kernel.org, Andrew Morton <akpm@linux-foundation.org>, Vlastimil Babka <vbabka@suse.cz>, rientjes@google.com, LKML <linux-kernel@vger.kernel.org>, Michal Hocko <mhocko@suse.cz>, Johannes Weiner <hannes@cmpxchg.org>, vdavydov@virtuozzo.com, Dan Williams <dan.j.williams@intel.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>, Chen Gang <gang.chen.5i5j@gmail.com> Subject: Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles Date: Sun, 28 Feb 2016 23:28:47 +0800 [thread overview] Message-ID: <56D3122F.1000802@emindsoft.com.cn> (raw) In-Reply-To: <20160228132717.GD2854@techsingularity.net> On 2/28/16 21:27, Mel Gorman wrote: > On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote: >> >> For me, NAK also needs reasons. >> > > You already got the reasons. Not only does a patch of this type interfere > with git blame which is important even in headers but I do not think the > patch actually improves the readability of the code. For example, the > comments move to the line after the defintions which to my eye at least > looks clumsy and weird. > For me, in local headers, they may be often modified, and also may be complex, so the code analyzing maybe also be used often. But in common shared headers in ./include (e.g. gfp.h), most of them are simple enough. - Since common shared headers are usually simple, code analyzing is still useful, but not like the body files or local headers (code analyzing are very useful for body files and local headers). - Common shared headers are quite often read by most programmers, so common shared headers need take more care about its coding styles. - Then for common shared headers, the coding style is 1st. And for __GFP_MOVABLE definition (with ZONE_MOVABLE), I guess, we can keep it no touch (like what I originally said: if the related member stick to, we can keep it no touch). And for me, the other macro definitions which out of 80 columns, can be fixed in normal ways (let the related comments ahead of macro definition ), does this change also have negative effect? >> I guess they are related with this patch, and their NAKs' reason are: mm >> and trivial don't care about this coding style issue, is it correct? >> > > No. Coding style is important but it's a guideline not a law. Yes. For me, vertical split window in vim is very useful, I almost always use this feature when read source code in full screen under Macbook client, when columns are 86+, it will be wrapped (I feel really not quite good). And occasionally (really not often), we may copy/past part of contents in the header files (e.g. constant definition) to the pdf file as appendix. So except the string broken, or "grep -rn xxx * | grep yyy", 80 columns limitation is always helpful to me. > There are > cases where breaking it results in perfectly readable code. At least one > my my own recent patches was flagged by checkpatch as having style issues > but fixing the style was considerably harder to read so I left it. If the > definitions in that header need to change again in the future and there > are style issues then they can be fixed in the context of a functional > change instead of patching style just for the sake of it. > For me, except just modify the related contents, usually, we need devide the patch into 2: one for real modification, the other for coding styles. And in some of common, base, shared headers in ./include (e.g. gfp.h), I guess, most of contents *should* not be changed quite often, so the bad coding styles probably will be alive in a long term. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
WARNING: multiple messages have this Message-ID (diff)
From: Chen Gang <chengang@emindsoft.com.cn> To: Mel Gorman <mgorman@techsingularity.net> Cc: Theodore Ts'o <tytso@mit.edu>, Jianyu Zhan <nasa4836@gmail.com>, trivial@kernel.org, Andrew Morton <akpm@linux-foundation.org>, Vlastimil Babka <vbabka@suse.cz>, rientjes@google.com, LKML <linux-kernel@vger.kernel.org>, Michal Hocko <mhocko@suse.cz>, Johannes Weiner <hannes@cmpxchg.org>, vdavydov@virtuozzo.com, Dan Williams <dan.j.williams@intel.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>, Chen Gang <gang.chen.5i5j@gmail.com> Subject: Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles Date: Sun, 28 Feb 2016 23:28:47 +0800 [thread overview] Message-ID: <56D3122F.1000802@emindsoft.com.cn> (raw) In-Reply-To: <20160228132717.GD2854@techsingularity.net> On 2/28/16 21:27, Mel Gorman wrote: > On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote: >> >> For me, NAK also needs reasons. >> > > You already got the reasons. Not only does a patch of this type interfere > with git blame which is important even in headers but I do not think the > patch actually improves the readability of the code. For example, the > comments move to the line after the defintions which to my eye at least > looks clumsy and weird. > For me, in local headers, they may be often modified, and also may be complex, so the code analyzing maybe also be used often. But in common shared headers in ./include (e.g. gfp.h), most of them are simple enough. - Since common shared headers are usually simple, code analyzing is still useful, but not like the body files or local headers (code analyzing are very useful for body files and local headers). - Common shared headers are quite often read by most programmers, so common shared headers need take more care about its coding styles. - Then for common shared headers, the coding style is 1st. And for __GFP_MOVABLE definition (with ZONE_MOVABLE), I guess, we can keep it no touch (like what I originally said: if the related member stick to, we can keep it no touch). And for me, the other macro definitions which out of 80 columns, can be fixed in normal ways (let the related comments ahead of macro definition ), does this change also have negative effect? >> I guess they are related with this patch, and their NAKs' reason are: mm >> and trivial don't care about this coding style issue, is it correct? >> > > No. Coding style is important but it's a guideline not a law. Yes. For me, vertical split window in vim is very useful, I almost always use this feature when read source code in full screen under Macbook client, when columns are 86+, it will be wrapped (I feel really not quite good). And occasionally (really not often), we may copy/past part of contents in the header files (e.g. constant definition) to the pdf file as appendix. So except the string broken, or "grep -rn xxx * | grep yyy", 80 columns limitation is always helpful to me. > There are > cases where breaking it results in perfectly readable code. At least one > my my own recent patches was flagged by checkpatch as having style issues > but fixing the style was considerably harder to read so I left it. If the > definitions in that header need to change again in the future and there > are style issues then they can be fixed in the context of a functional > change instead of patching style just for the sake of it. > For me, except just modify the related contents, usually, we need devide the patch into 2: one for real modification, the other for coding styles. And in some of common, base, shared headers in ./include (e.g. gfp.h), I guess, most of contents *should* not be changed quite often, so the bad coding styles probably will be alive in a long term. Thanks. -- Chen Gang (e??a??) Managing Natural Environments is the Duty of Human Beings. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-02-28 15:25 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-02-24 22:26 [PATCH trivial] include/linux/gfp.h: Improve the coding styles chengang 2016-02-24 22:26 ` chengang 2016-02-25 1:01 ` SeongJae Park 2016-02-25 1:01 ` SeongJae Park 2016-02-25 14:12 ` Chen Gang 2016-02-25 14:12 ` Chen Gang 2016-02-25 8:57 ` Michal Hocko 2016-02-25 8:57 ` Michal Hocko 2016-02-25 14:23 ` Chen Gang 2016-02-25 14:23 ` Chen Gang 2016-02-25 14:47 ` Michal Hocko 2016-02-25 14:47 ` Michal Hocko 2016-02-25 22:17 ` Chen Gang 2016-02-25 22:17 ` Chen Gang 2016-02-25 9:27 ` Mel Gorman 2016-02-25 9:27 ` Mel Gorman 2016-02-25 14:38 ` Chen Gang 2016-02-25 14:38 ` Chen Gang 2016-02-25 15:12 ` Jiri Kosina 2016-02-25 15:12 ` Jiri Kosina 2016-02-25 22:19 ` Chen Gang 2016-02-25 22:19 ` Chen Gang 2016-02-25 16:07 ` Mel Gorman 2016-02-25 16:07 ` Mel Gorman 2016-02-25 22:29 ` Chen Gang 2016-02-25 22:29 ` Chen Gang 2016-02-25 22:39 ` Jiri Kosina 2016-02-25 22:39 ` Jiri Kosina 2016-02-26 14:57 ` Chen Gang 2016-02-26 14:57 ` Chen Gang 2016-02-25 23:12 ` SeongJae Park 2016-02-25 23:12 ` SeongJae Park 2016-02-26 15:06 ` Chen Gang 2016-02-26 15:06 ` Chen Gang 2016-02-26 2:32 ` Jianyu Zhan 2016-02-26 2:32 ` Jianyu Zhan 2016-02-26 15:26 ` Chen Gang 2016-02-26 15:26 ` Chen Gang 2016-02-27 2:45 ` Theodore Ts'o 2016-02-27 2:45 ` Theodore Ts'o 2016-02-27 14:32 ` Chen Gang 2016-02-27 14:32 ` Chen Gang 2016-02-27 16:53 ` Theodore Ts'o 2016-02-27 16:53 ` Theodore Ts'o 2016-02-28 0:21 ` Chen Gang 2016-02-28 0:21 ` Chen Gang 2016-02-28 13:27 ` Mel Gorman 2016-02-28 13:27 ` Mel Gorman 2016-02-28 15:28 ` Chen Gang [this message] 2016-02-28 15:28 ` Chen Gang 2016-02-27 23:14 ` Jiri Kosina 2016-02-27 23:14 ` Jiri Kosina 2016-02-28 0:47 ` Chen Gang 2016-02-28 0:47 ` Chen Gang 2016-02-28 22:23 ` Theodore Ts'o 2016-02-29 15:48 ` Chen Gang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56D3122F.1000802@emindsoft.com.cn \ --to=chengang@emindsoft.com.cn \ --cc=akpm@linux-foundation.org \ --cc=dan.j.williams@intel.com \ --cc=gang.chen.5i5j@gmail.com \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@techsingularity.net \ --cc=mhocko@suse.cz \ --cc=nasa4836@gmail.com \ --cc=rientjes@google.com \ --cc=trivial@kernel.org \ --cc=tytso@mit.edu \ --cc=vbabka@suse.cz \ --cc=vdavydov@virtuozzo.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.