-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3643/#review8772
-----------------------------------------------------------

Ship it!


Seems reasonable to me. Could you explain in the commit message how you decided 
which headers to remove/keep/add? Without digging into these files it's hard to 
understand your thought process.

Related: Do you think it makes sense to codify which headers to include in the 
style guideline? IMO, this would be helpful so you don't have to go through and 
do this again :).

One other small thing. I don't think we should hold up this patch for this 
issue, but it's something that's been bothering me. It seems the style checker 
is complaining about the ordering of includes for this patch, though I don't 
think they are actually wrong. When I run the following I get errors:

```
hg qref COPYING # to get the patch to be "unapplied" but the changes reflected 
to the files
util/style.py 
... lots of errors. For every single file it complains about the ordering of 
includes.
```

If I just use hg qref, the style checker doesn't complain. Ideas?

- Jason Lowe-Power


On Oct. 5, 2016, 4:27 p.m., Brandon Potter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> -----------------------------------------------------------
> 
> (Updated Oct. 5, 2016, 4:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11660:1c76ab8bafec
> ---------------------------
> style: reduce include dependencies in some headers
> 
> 
> Diffs
> -----
> 
>   src/arch/x86/system.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table_impl.hh 
> b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/arguments.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/generic/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/power/interrupts.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pseudo_inst.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/system.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
> 
> Diff: http://reviews.gem5.org/r/3643/diff/
> 
> 
> Testing
> -------
> 
> util/regress
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to