Hi Zihang,
I just had a brief glance through the whole commit. That's a very large change! It's essentially the same code with lots of moving around and cosmetic changes. However, I do have a couple of issues with it: 1. I found it really difficult to follow the code. You should edit the README file to reflect the current scenario and how should a developer follow it. 2. It seems like you've created some really nice abstractions, it would very nice to explain them so the developers for Wget know what to look at and what to edit. 3. While the code surely is more pythonic, it creates a slight problem. It's *more* pythonic. Most people who have to deal with this code are not users who use Python everyday. I think, a little lesser of strict Python syntax and a little more of simpler syntax will allow non-Python developers to more easily follow the code. The point of using Python to rewrite the old test suite was that Perl was a bit too cryptic and people had to spend too much time understanding the code first before they could edit it. I don't want to repeat that with having truly pythonic code which takes more time to follow for a C developer. Others, please chime in on this. I like the overall restructuring though. And if the abstractions do work the way I think they do, I believe this could be a good idea. I'll look at it in much more detail when I get the time. On Sun, Mar 9, 2014 at 2:22 AM, Darshit Shah <[email protected]> wrote: > Hi, > > Thanks for the refactoring. However, you've included makefile and > makefile.in which are autogenerated files and should not be commited. > > Also, your patch has trailing whitespace errors. And I don't think you've > added an entry to ChangeLog either. Please look into these. I haven't seen > your patch yet. The Makefile errors mean I can't apply it without a lot of > extra work. > > > On Sat, Mar 8, 2014 at 5:38 PM, 陈子杭 (Zihang Chen) <[email protected]>wrote: > >> Hi. >> >> Here's a patch refactoring CommonMethods, HTTPTest and project hierarchy. >> 19 tests passed, 1 xfailed as expected. >> >> Let me know if there's any problem. >> >> -- >> Regards, >> Chen Zihang, >> Computer School of Wuhan University >> --- >> 此致 >> 陈子杭 >> 武汉大学计算机学院 >> > > > > -- > Thanking You, > Darshit Shah > > -- Thanking You, Darshit Shah
