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: Mon, 10 Mar 2014 17:25:30 +0800

I applied dos2unix to all the files under testenv, checked with file
command, deleted all pyc files, line wrap to 80 characters and format a new
patch. (I swear this will be the last huge patch I'll ever make.)

I also git am this patch to a clean clone locally, and got two warning:
    warning: squelched 16 whitespace errors
    warning: 21 lines add whitespace errors.
Is this ok?

Nervously, Chen


2014-03-10 16:34 GMT+08:00 陈子杭 (Zihang Chen) <address@hidden>:

>
>
>
> 2014-03-10 16:17 GMT+08:00 Darshit Shah <address@hidden>:
>
>
>>
>>
>> On Mon, Mar 10, 2014 at 8:46 AM, 陈子杭 (Zihang Chen) <address@hidden>wrote:
>>
>>> Hi Yousong,
>>>
>>> So sorry about the line endings, I'll have to do a thorough check.
>>>
>> I'm not sure about the line endings since my git and vim cinfiguration
>> simply do the magic
>> of conversions for me. But if Yousong says do, do look into it.
>>
>> However, you seem to have added a huge amount of those especially in your
>> 2nd patch.
>>
>> I do however, very strongly suggest that you get access to some sort of a
>> linux system. It will
>> make your life so much easier. Autoconf takes ages to run on Windows in a
>> cygwin shell.
>>
>>
>>>
>>> BTW, the pyc files in 0001.patch was deleted in the second commit.
>>>
>>
>> It would be better if you just did not have them there. It woulld clutter
>> *everyone's* git repos
>> if the .pyc files were there and later deleted. Because git will leave a
>> snapshot of each
>> commit in the history. Keep a .gitignore file handy. Those are very
>> important. You'll get
>> good ones for starts from github's own gitignore repository.
>>
> Got it. But I wonder where to put the .gitignore file. Should I use the
> one in the `wget` directory or
> get a new one under `testenv`?
>
>>
>>
>> Also, we usually expect a ChangeLog entry for *every* patch being sent.
>> So, please keep that
>> in mind too. And there's also the 80 characters per line limit we like to
>> follow for all files.
>>
>> I'll keep that in mind.
>
>> The chief reason was that older terminals could only display 80
>> characters. Now, the reason is
>> that it allows you to have two vertical windows with code simultaneously
>> without any line wraps.
>>
>> And do follow Yousong's advice on organizing your patchset. Ask for help
>> and you shall get it.
>> Large, single commits are seldom looked upon favourably.
>>
>>>
>>> I'll try to make my commits smaller next time. Work till now I is not
> likely to be divided into small
> commits though ;(
>
>>  And thanks very much for the advice!
>>>
>>>
>>> 2014-03-10 15:38 GMT+08:00 Yousong Zhou <address@hidden>:
>>>
>>> > Hi, Zihang,
>>> >
>>> > On 10 March 2014 13:05, 陈子杭 (Zihang Chen) <address@hidden> wrote:
>>> > > Hi, Darshit.
>>> > > I fixed the line ending using git config --global autocrlf input.
>>> Line
>>> > > endings should be lf now. I also added some documentation. File
>>> modes for
>>> > > Test-*.py are 755 now.
>>> > >
>>> >
>>> > I just did a quick check on the patch and the line endings are still
>>> > wrong, e.g. testenv/test/http_test.py
>>> >
>>> > Also, .pyc files should not be included, right?
>>> >
>>> > I do not have much experience with parallel-wget, but you can enhance
>>> > organizing your commits by following how existing ones in the
>>> > repository were written.
>>> >
>>> >
>>> >                yousong
>>> >
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Chen Zihang,
>>> Computer School of Wuhan University
>>> ---
>>> 此致
>>> 陈子杭
>>> 武汉大学计算机学院
>>>
>>
>>
>>
>> --
>> Thanking You,
>> Darshit Shah
>>
>>
>
>
> --
> Regards,
> Chen Zihang,
> Computer School of Wuhan University
> ---
> 此致
> 陈子杭
> 武汉大学计算机学院
>
>


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

Attachment: 0001-refactor-test-case-classes-and-add-documentation.patch
Description: Text Data


reply via email to

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