Ah, I see the problem. The real problem is, we shouldn't use diamond include instead of quotes include for project header, which is cheating cpplint to consider them as system header. Diamond include should only be used for system header, including OS header, STL header and host-installed library header(opencv for example). For others, quote include should be used.
Thanks, Zhennan -----Original Message----- From: Pedro Larroy [mailto:pedro.larroy.li...@gmail.com] Sent: Wednesday, January 9, 2019 9:29 AM To: dev@mxnet.incubator.apache.org Subject: Re: Order of includes in cpplint I worked around the case, I saw several cases where project and third party headers were included with diamond includes instead of quotes. Agree I think is too much hassle to change the order of includes in the whole project anyway. But this convention can cause some headers not to be self sufficient as system headers are being included before other library headers which might be using a system header like <string> without including it themselves. Pedro. On Wed, Jan 9, 2019 at 2:12 AM Qin, Zhennan <zhennan....@intel.com> wrote: > > Hi Pedro, > > Interesting topic. Google style does have guidance for this: > > https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_I > ncludes > > According to it, the order is, > > dir2/foo2.h. > A blank line > C system files. > C++ system files. > A blank line > Other libraries' .h files. > Your project's .h files. > > As MXNet follows this style, I guess we shouldn't break it unless we have > some problems. Do you have such a case that need the change? > > Thanks, > Zhennan > > -----Original Message----- > From: Pedro Larroy [mailto:pedro.larroy.li...@gmail.com] > Sent: Wednesday, January 9, 2019 6:44 AM > To: dev@mxnet.incubator.apache.org > Subject: Order of includes in cpplint > > Hi MXNet community > > cpplint seems to complain when the order of includes is not [own, > system, other] > > But the general best practice in C++ is [own, project, 3rd party, > system] for the reasons explained in this stackoverflow answer: ( > https://stackoverflow.com/questions/614302/c-header-order ) > > A contribution to cpplint could be made to make this configurable: > > https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605 > > Thoughts? > > Pedro.
