[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] [GSoC] Refactoring the Test Suite

From: Zihang Chen
Subject: Re: [Bug-wget] [GSoC] Refactoring the Test Suite
Date: Sun, 9 Mar 2014 11:34:45 +0800

2014-03-09 9:39 GMT+08:00 Darshit Shah <address@hidden>:

> 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.
I'm most sorry for the lack of documentation. I'll try adding some to
README, CHANGELOG and the code itself (though I think some of the code is
self-explaining itself, I'll add comments for non-pythoners, sorry again.)

> 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 <address@hidden> 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) <address@hidden>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

Chen Zihang,
Computer School of Wuhan University

reply via email to

[Prev in Thread] Current Thread [Next in Thread]