----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2614/#review5804 -----------------------------------------------------------
This is a welcome change. We're still finessing includes-ordering in gem5-gpu, and this change subsumes some of the unofficial policies we're following. Just a couple minor code style comments below. util/sort_includes.py <http://reviews.gem5.org/r/2614/#comment5122> The Python "do X if Y else Z" conditional formatting tends to be unclear and tricky to read, as is the case here. It's also quite uncommon in gem5 (in fact, I'm unable to quickly find another example in the codebase), and using it in line 61 and here mixes code style with the more standard use in analogous line 100. My preference would be to maintain the clearer and consistent use of if-conditionals already common in gem5. util/sort_includes.py <http://reviews.gem5.org/r/2614/#comment5123> This and lines 154+155 are tricky to read also. The old code was much clearer/easier to step through. Is it important for the code to be this dense? - Joel Hestness On Jan. 26, 2015, 5:55 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2614/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 5:55 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10678:f4e8410ba60e > --------------------------- > style: Update the style checker to handle new include order > > As of August 2014, the gem5 style guide mandates that a source file's > primary header is included first in that source file. This helps to > ensure that the header file does not depend on include file ordering > and avoids surprises down the road when someone tries to reuse code. > > In the new order, include files are grouped into the following blocks: > * Primary header file (e.g., foo.hh for foo.cc) > * Python headers > * C system/stdlib includes > * C++ stdlib includes > * Include files in the gem5 source tree > > Just like before, include files within a block are required to be > sorted in alphabetical order. > > This changeset updates the style checker to enforce the new order. > > > Diffs > ----- > > util/sort_includes.py 3c42be107634 > util/style.py 3c42be107634 > > Diff: http://reviews.gem5.org/r/2614/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
