Hi Darshit and Yousong, I think I finally got things straight.
The big commit was split into 9 relatively small commits, and I removed all the trailing backspaces and new lines. These patches apply to origin/parallel-wget without warnings. Thank both of you very much for all the help! If you have any concerns about the contents of the patches, please let me know. 2014-03-10 19:49 GMT+08:00 Darshit Shah <[email protected]>: > > > > On Mon, Mar 10, 2014 at 10:25 AM, 陈子杭 (Zihang Chen) <[email protected]>wrote: > >> 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? >> >> I haven't checked the patch yet, but just a few suggestions: > > 1. You don't need to delete the pyc files locally. Simply don't add them > to the git commit. Use a local .gitignore file to handle it > 2. You can and should split this patch. I'm assuming it's the same stuff > as before, and that can be split. Use your imagination > 3. The whitespace errors imply trailing whitespace. This happens when yo > uhave extra whitespace characters at the end of a > line. Usually not a good idea sinec these are characters that cannot be > seen. You should eliminate them. My ViM editor > simply highlights all trailing whitespaces so I always know if they are > there. Also, you can configure your git to explicitly > highlight trailing whitespaces in its diff output (Assuming you're using a > git shell, not a GUI, in which case I have no idea.) > > Nervously, Chen >> > Don't worry. Everyone faces problems with these items in the beginning. > It's not something you are used to. > > >> >> >> 2014-03-10 16:34 GMT+08:00 陈子杭 (Zihang Chen) <[email protected]>: >> >> >>> >>> >>> 2014-03-10 16:17 GMT+08:00 Darshit Shah <[email protected]>: >>> >>> >>>> >>>> >>>> On Mon, Mar 10, 2014 at 8:46 AM, 陈子杭 (Zihang Chen) >>>> <[email protected]>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 <[email protected]>: >>>>> >>>>> > Hi, Zihang, >>>>> > >>>>> > On 10 March 2014 13:05, 陈子杭 (Zihang Chen) <[email protected]> >>>>> 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 >> --- >> 此致 >> 陈子杭 >> 武汉大学计算机学院 >> >> > > > -- > Thanking You, > Darshit Shah > > -- Regards, Chen Zihang, Computer School of Wuhan University --- 此致 陈子杭 武汉大学计算机学院
patches.tar.gz
Description: GNU Zip compressed data
