On Mon, Dec 21, 2009 at 9:03 AM, nathan binkert <[email protected]> wrote: > Overall, things look pretty good. There are more or less four things > that you've done that need work. > > 1) You created SimObjects for all of the ruby objects and in the > process, you didn't give any help strings to any of the parameters. > While I understand that this can be tedious, this is one minimal form > of documentation that does help users. > 2) You removed Garnet from the compile. Do we want to keep garnet? > If so, this is guaranteed to lead to bit rot. > 3) You commented out code instead of removing it. This should only > *very* rarely be done. The old code is in the history, so just delete > it. > 4) You've added #includes in places, but have not maintained sorted > order. It's a lot easier to visually scan #includes if they're all > sorted. This lets you easily find duplicates and quickly figure out > if you need to add something. (There's something in the coding style > document about this.)
I'm responsible for 1 & 2 and probably partially responsible for 4. (Yes, we still need to clean up the individual patch attributions.) I did the initial SimObject conversion and was just trying to get something that compiled, which is why I bailed on the parameter description strings and on compiling Garnet. (To be honest I don't remember to what extent there were existing parameter description strings that didn't get copied over vs. not being any in the first place.) Then once I got to a certain point where things compiled but didn't quite work (basically all the Ruby objects were getting constructed but they weren't hooked up right) I handed it over to Brad since he understood that part better. I guess we both forgot about going back and taking care of those temporary shortcuts. Steve _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
