I have a few more comments. (Steve caught the major ones) -- Please don't commit random whitespace changes as this makes following history more difficult. If you just review your diffs, they're easy to spot. -- Please include commit messages with your patches. (The -m option to qrefresh can be used to add them.) -- You don't need to put "# -*- mode:python -*-" at the top of files that end in .py. You should put them at the top of SConscript and SConsopt files.
If you are willing to make your patches available in a repository somewhere, I'm sure that we could help you clean things up. (I can take care of copyright stuff). I'd also like to change the name from mixie to inorder. Those are two things I can do if you give me a few hours to own your patches. Nate 2009/1/17 Korey Sewell <ksew...@umich.edu>: > Thanks Steve, > looks like I have some re-work todo. > > My original repo got corrupted when I try to pull in changes without > updating my patches. I was forced to import my patches into a new repo and I > bet what happened was things got messed up when I hand-merged things over. > > arrrrgh.... > > OK, I'll take a look at remedying all of the issues you outline below on > Monday when I get back to this stuff. > > 2009/1/17 Steve Reinhardt <ste...@gmail.com> >> >> Hi Korey, >> >> Thanks for sending these out! I just did a quick skim; here are a few >> initial comments based on that: >> >> - What's up with some of the license changes? It looked to me like on >> some of the files you're changing the license back from the new style (where >> U-M is mentioned only at the top) to the old style (with U-M mentioned in >> the body of the license). >> >> - For the cases where you're committing a new file, it should be committed >> with a license in place. >> >> - Don't commit changes that just comment code out. If the code doesn't >> belong any more, delete it. If the code should be there, but there's a >> reason why it can't be there right now, fixing the problem would be best. >> The least desirable but minimally acceptable approach would be to put the >> code in a larger comment (either /* */ or #if 0 #endif) and then put further >> comments inside of that explaining why it's commented out and what needs to >> be done before it can be uncommented. >> >> - Since this is mostly a big import anyway, I'd favor integrating all the >> outstanding bug fixes in up front. For example there's one patch that >> introduces a memAccFlags() method that returns unsigned, then another patch >> that fixes the return value to Request::Flags... why not just import it >> right in the first place. >> >> Steve >> >> On Sat, Jan 17, 2009 at 7:44 AM, Korey Sewell <ksew...@eecs.umich.edu> >> wrote: >>> >>> Series of patches imports the mixie in-order model and gets it working >>> for a MIPS hello-world application. >>> >>> Next up, more serious regression testing. >>> _______________________________________________ >>> m5-dev mailing list >>> m5-dev@m5sim.org >>> http://m5sim.org/mailman/listinfo/m5-dev >> >> >> _______________________________________________ >> m5-dev mailing list >> m5-dev@m5sim.org >> http://m5sim.org/mailman/listinfo/m5-dev >> > > > > -- > ---------- > Korey L Sewell > Graduate Student - PhD Candidate > Computer Science & Engineering > University of Michigan > > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
