Closed cpkleynhans closed 11 years ago
Two minor points: (currently) unnecessary import and trailing whitespace in various places.
We can ignore the minor points for now.
I also have two major comments:
waf
. So either download everything or include everything.I've fixed your minor points.
In order for us to download waf we would need to add another script or something to download dependencies. This seems a bit excessive so for waf we have two decisions really, include it in the repo, or assume the user has it installed already. I prefer including it in the repo.
Overflow appears to be using an older version of junit, so we would need to update the tests to the latest version. The tests are also organized in a slightly different way but it shouldn't be hard to adjust them.
OK, looks good then.
looks good!
We discussed offline that we should move test building into its own task. I'll be pushing a commit with this change shortly.
Changes:
Reviewers: @AtulanZaman @mhyee @joseph2625