bug-wget
[Top][All Lists]
Advanced

[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:38:28 +0800

2014-03-09 11:31 GMT+08:00 Yousong Zhou <address@hidden>:

> Hi, Zihang and Darshit, and all.
>
> On 9 March 2014 09:39, Darshit Shah <address@hidden> wrote:
> > 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.
>
> Hi, Zihang. The patch is really big as a single commit.  You'd better
> split it into multiple small ones each for a single purpose, without
> breaking the code with each commit if possible.  That way we can refer
> to and comment on the code more easily.
>
> > 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.
>
> To be honest, I am fine with the current Perl implementation.  My last
> several patches for the Perl-based test framework are my first try
> with Perl.  It does not take me much time to understand the design and
> modify a few lines of code.  I think documentation or self-explaining
> code is the solution.
>
> >
> > 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.
> >>
>
> Hi Zihang, looks like you development environment has to be configured
> right for this.  The newline character in your code in now '\r\n'
> which should be '\n'.

Yes, you're right. I mostly work under Windows. Didn't notice that. My bad.
BTW is there anything similar to guideline for setting the development
environment? Please let me know if there is one.
Thanks, Yousong.

>  There are mode changes from 100755 to 100644 in
> the git commit, which is not right.  Those .py files should retain
> their executable attributes.
>
>
>                yousong
>



-- 
Regards,
Chen Zihang,
Computer School of Wuhan University
---
此致
陈子杭
武汉大学计算机学院


reply via email to

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