> On 2011-08-09 09:07:14, Brad Beckmann wrote:
> > I just have a couple minor edits to make to the comments.  However, I 
> > wonder what is the real benefit for combining the two fs files.  I don't 
> > believe there is a lot of duplicate code between the two.  Most of the 
> > added options in fs.py are not supported/related to Ruby.  Also with your 
> > change, the main body of fs.py is now a large if/else statement with the if 
> > being the functionality from ruby_fs.py and the else being the fs.py 
> > functionality.  Overall, I feel this is an unnecessary change that will 
> > negatively impact those of us who have other internal patches that modify 
> > these files.  Why do it?
> >

There are a couple of reasons. We should not have different set of options for
ruby_fs.py and fs.py. Since we have already combined se.py and ruby_se.py, we
should do the same for fs scripts as well. The main body of fs.py being a large
if/else is either because I have not taken out all those things that are common,
and that we still lack in the integration that Ruby and M5 should achieve. As 
far as internal patches are concerned, unless the patches can be updated, they 
should be used with the version with which they were created. Going ahead, we 
are only going to add more features, sooner or later these features will step
on the code used in the patches.


- Nilay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/818/#review1452
-----------------------------------------------------------


On 2011-08-06 00:43:50, Nilay Vaish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/818/
> -----------------------------------------------------------
> 
> (Updated 2011-08-06 00:43:50)
> 
> 
> Review request for Default.
> 
> 
> Summary
> -------
> 
> Config: Add ruby_fs.py's functionality to fs.py
> This patch combines fs.py and ruby_fs.py and removes ruby_fs.py. Using
> ruby with fs.py would require mentioning --ruby on the command line. This
> same as se.py.
> 
> 
> Diffs
> -----
> 
>   configs/example/fs.py 7a9a7f2a3d46 
>   configs/example/ruby_fs.py 7a9a7f2a3d46 
> 
> Diff: http://reviews.m5sim.org/r/818/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nilay
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to