With Zihang's copyright assignment process over, is there anything that is still blocking the process of merging this set of patches?
On Tue, Apr 15, 2014 at 11:33 AM, 陈子杭 (Zihang Chen) <[email protected]> wrote: > Hi all. > > I here upload v3 of the patch series. > > Changes: > * Fix no new line endings, which eliminates patch10 of v2 > * README is updated along the patches, which eliminates patch9 of v2 > * patch11 of v2 is merged into patch5 of v3 > * Fix various inappropriateness metions in previous mail > > Speaking about the overhead of loading conf package (which has a lot of > small scripts), I timed `make check` and the real time of the original test > suite is 15.129s and that of patch-applied test suite is 16.786s, on first > sight the original is faster (about 2s). > I still prefer small scripts, mainly because of their extensibility and > maintainability. The reason why the latter test suite is slower, I think, is > that `make check` create a new Python process for every test case (which > does not take advantage of Python's import mechanism), and that's not what > modern test suites do. I'd suggest that we make a bootstrap Python script, > which makes sure that only one Python process is created during test. And > make `make check` use that script to do the tests. > > Thanks in advance and cheers. > > Zihang Chen (8): > Create package misc, move ColourTerm.py to misc > From WgetTest.py move WgetFile to misc > Fix a typo in Test-Proto.py > Create package exc and move TestFailed to exc > Create package conf where rules and hooks are put > Move server classes to package server.protocol > Create package test for test case classes > Refactor mainly the test cases classes > > testenv/ChangeLog | 143 ++++++++ > testenv/ColourTerm.py | 23 -- > testenv/FTPServer.py | 162 --------- > testenv/HTTPServer.py | 467 > -------------------------- > testenv/README | 77 +++-- > testenv/Test--https.py | 6 +- > testenv/Test--spider-r.py | 3 +- > testenv/Test-Content-disposition-2.py | 3 +- > testenv/Test-Content-disposition.py | 3 +- > testenv/Test-Head.py | 3 +- > testenv/Test-O.py | 3 +- > testenv/Test-Parallel-Proto.py | 6 +- > testenv/Test-Post.py | 3 +- > testenv/Test-Proto.py | 6 +- > testenv/Test-auth-basic-fail.py | 3 +- > testenv/Test-auth-basic.py | 3 +- > testenv/Test-auth-both.py | 3 +- > testenv/Test-auth-digest.py | 3 +- > testenv/Test-auth-no-challenge-url.py | 3 +- > testenv/Test-auth-no-challenge.py | 3 +- > testenv/Test-auth-retcode.py | 3 +- > testenv/Test-auth-with-content-disposition.py | 3 +- > testenv/Test-c-full.py | 3 +- > testenv/Test-cookie-401.py | 3 +- > testenv/Test-cookie-domain-mismatch.py | 3 +- > testenv/Test-cookie-expires.py | 3 +- > testenv/Test-cookie.py | 3 +- > testenv/WgetTest.py | 337 ------------------- > testenv/conf/__init__.py | 47 +++ > testenv/conf/authentication.py | 9 + > testenv/conf/expect_header.py | 7 + > testenv/conf/expected_files.py | 42 +++ > testenv/conf/expected_ret_code.py | 16 + > testenv/conf/files_crawled.py | 18 + > testenv/conf/hook_sample.py | 15 + > testenv/conf/local_files.py | 12 + > testenv/conf/reject_header.py | 7 + > testenv/conf/response.py | 7 + > testenv/conf/rule_sample.py | 10 + > testenv/conf/send_header.py | 7 + > testenv/conf/server_conf.py | 11 + > testenv/conf/server_files.py | 15 + > testenv/conf/urls.py | 10 + > testenv/conf/wget_commands.py | 10 + > testenv/exc/__init__.py | 1 + > testenv/exc/test_failed.py | 7 + > testenv/misc/__init__.py | 1 + > testenv/misc/colour_terminal.py | 31 ++ > testenv/misc/wget_file.py | 16 + > testenv/server/__init__.py | 1 + > testenv/server/ftp/__init__.py | 1 + > testenv/server/ftp/ftp_server.py | 162 +++++++++ > testenv/server/http/__init__.py | 1 + > testenv/server/http/http_server.py | 467 > ++++++++++++++++++++++++++ > testenv/test/__init__.py | 1 + > testenv/test/base_test.py | 226 +++++++++++++ > testenv/test/http_test.py | 45 +++ > 57 files changed, 1451 insertions(+), 1036 deletions(-) > delete mode 100644 testenv/ColourTerm.py > delete mode 100644 testenv/FTPServer.py > delete mode 100644 testenv/HTTPServer.py > delete mode 100644 testenv/WgetTest.py > create mode 100644 testenv/conf/__init__.py > create mode 100644 testenv/conf/authentication.py > create mode 100644 testenv/conf/expect_header.py > create mode 100644 testenv/conf/expected_files.py > create mode 100644 testenv/conf/expected_ret_code.py > create mode 100644 testenv/conf/files_crawled.py > create mode 100644 testenv/conf/hook_sample.py > create mode 100644 testenv/conf/local_files.py > create mode 100644 testenv/conf/reject_header.py > create mode 100644 testenv/conf/response.py > create mode 100644 testenv/conf/rule_sample.py > create mode 100644 testenv/conf/send_header.py > create mode 100644 testenv/conf/server_conf.py > create mode 100644 testenv/conf/server_files.py > create mode 100644 testenv/conf/urls.py > create mode 100644 testenv/conf/wget_commands.py > create mode 100644 testenv/exc/__init__.py > create mode 100644 testenv/exc/test_failed.py > create mode 100644 testenv/misc/__init__.py > create mode 100644 testenv/misc/colour_terminal.py > create mode 100644 testenv/misc/wget_file.py > create mode 100644 testenv/server/__init__.py > create mode 100644 testenv/server/ftp/__init__.py > create mode 100644 testenv/server/ftp/ftp_server.py > create mode 100644 testenv/server/http/__init__.py > create mode 100644 testenv/server/http/http_server.py > create mode 100644 testenv/test/__init__.py > create mode 100644 testenv/test/base_test.py > create mode 100644 testenv/test/http_test.py > > -- > 1.8.3.2 > > > > 2014-04-10 15:56 GMT+08:00 陈子杭 (Zihang Chen) <[email protected]>: > >> >> >> >> 2014-04-08 7:24 GMT+08:00 Darshit Shah <[email protected]>: >>> >>> Hi, >>> >>> I'm going to be nitpicking, so bear with me: >>> >>> 1. See code [1]. Why is the import statement for HTTPServer being moved? >>> I don't see any functional use of this. >> >> My fault, sorry. PyCharm (the IDE I use) sometimes does something kinky >> silently, especially when it optimizes the imports automatically. >>> >>> 2. The colour_teminal.py file you create has no newline at the end. This >>> was pointed out earlier too if I remember correctly. >> >> Yes. My fault too. I thought PyCharm has taken care of it for me. >>> >>> 3. I'm no longer listing multiple "No newline at end of file" issues. I'm >>> going to assume you'll hunt them all down and fix them. >> >> Sure. >>> >>> 4. This looks all valid and pythonic, but why are we doing all this >>> again? Is there some logical reason why splitting everything into so many >>> files is important? I'd say when things are small and manageable, why not >>> let it all rest in a single file? I'm no >> >> Of course they can be in one file. However I was afraid that the file >> would end up too large. And what if there are more rules to be added? Maybe >> I overthought it. >>> >>> expert with Python, but this just seems like overkill to me. I'm looking >>> at patch 5, and you've got loads of new files which are all 10 lines each, >>> including import statements and comments. Why can't we use a single file for >>> them all? >>> In fact, I'd say, that with a scripting language this is highly >>> inefficient since you're now reading lot sof files from disk again and again >>> and again. >> >> Well, I have to point out that when these scripts are imported, Python >> actually compile them into byte codes and loads the byte codes into memory. >> So the overhead of importing is not that much. >>> >>> 5. I haven't thoroughly looked through patch 5 yet. I'd honestly like to >>> discuss and understand why so many files need to be created before moving on >>> from here. >>> 6. Again, I don't see any reason why you'd create a new file with only 2 >>> lines on code and will be used only once. [2] >> >> I admit that I overthought this after you pointing it out. >>> >>> 7. See [3]. Why does it have to be '../'. Isn't that non-portable syntax? >>> Since Windows happens to use \ for directories. Or did I miss something? >> >> My fault three. >>> >>> 8. A lot of your patches seem redundant editing the same file twice in >>> the same location. Why not just do it once? You first split things into >>> constants.py and imported them. Then, two patches later you change the >>> import statements. >>> 9. I'd suggest you update the README file along with each patch that >>> makes changes so that we never have a stale README version. >> >> Reasonable. I'll do that. >>> >>> 10. Patch 10 is purely aesthetic. In fact, so aesthetic, that it has no >>> changes to the naked, untrained eye. A full patch that only adds >>> whitespaces? Why not just fix them in the earliest patch causing the issue? >> >> My fault four. >> >> I have to admit that I was a bit lazy when doing the patches, which leads >> to so many stupid mistakes. Sorry for wasting your time and thanks for >> pointing these out. I will try to overcome my laziness and do my patches >> more carefully :) >>> >>> >>> P.S.: If you're going to send multiple versions of such a long patchset, >>> it would be a really good idea to also tell us what has changed between the >>> last version and this. That will allow us to look at the right places >>> better. >> >> Absolutely. >>> >>> >>> [1]: >>>> >>>> import shlex >>>> import sys >>>> import traceback >>>> -import HTTPServer >>>> import re >>>> import time >>>> from subprocess import call >>>> -from ColourTerm import printer >>>> from difflib import unified_diff >>>> >>>> +import HTTPServer >>>> +from misc.colour_terminal import print_red, print_green, print_blue >>>> + >>> >>> >>> [2]: >>>> >>>> diff --git a/testenv/misc/constants.py b/testenv/misc/constants.py >>>> new file mode 100644 >>>> index 0000000..5fad2f8 >>>> --- /dev/null >>>> +++ b/testenv/misc/constants.py >>>> @@ -0,0 +1,3 @@ >>>> + >>>> +HTTP = "HTTP" >>>> +HTTPS = "HTTPS" >>>> >>> >>> [3]: >>>> >>>> - CERTFILE = os.path.abspath (os.path.join ('..', 'certs', >>>> 'wget-cert.pem')) >>>> + CERTFILE = os.path.abspath (os.path.join ('../', 'certs', >>>> 'wget-cert.pem')) >>> >>> >>> >>> >>> >>> On Tue, Apr 1, 2014 at 9:41 AM, 陈子杭 (Zihang Chen) <[email protected]> >>> wrote: >>>> >>>> Thanks very much in advance for paying attention to this newbie patch >>>> series! >>>> >>>> >>>> 2014-04-01 15:39 GMT+08:00 陈子杭 (Zihang Chen) <[email protected]>: >>>> >>>> > Hi everyone. >>>> > >>>> > Sorry for being late on amending my previous patches. >>>> > >>>> > I here upload version 2 of the patches. >>>> > >>>> > Changes since v1: >>>> > * Uppercase git commit lines in order to be consist with the older >>>> > commits. >>>> > * Reduce lengths of commit lines and items in ChangeLog. >>>> > >>>> > P.S. I chose not to use git-send-email, it's too slow. >>>> > >>>> > Zihang Chen (11): >>>> > Create package misc, move ColourTerm.py to misc >>>> > From WgetTest.py move WgetFile to misc >>>> > Fix a typo in Test-Proto.py >>>> > Create package exc and move TestFailed to exc >>>> > Create package conf where rules and hooks are put >>>> > Move server classes to package server.protocol >>>> > Create package test for test case classes >>>> > Refactor mainly the test cases classes >>>> > Update README >>>> > Ensure line feed and blank line between methods >>>> > In conf, rename register to rule and hook >>>> > >>>> > testenv/ChangeLog | 144 ++++++++ >>>> > testenv/ColourTerm.py | 23 -- >>>> > testenv/FTPServer.py | 162 --------- >>>> > testenv/HTTPServer.py | 467 >>>> > -------------------------- >>>> > testenv/README | 81 +++-- >>>> > testenv/Test--https.py | 6 +- >>>> > testenv/Test--spider-r.py | 3 +- >>>> > testenv/Test-Content-disposition-2.py | 3 +- >>>> > testenv/Test-Content-disposition.py | 3 +- >>>> > testenv/Test-Head.py | 3 +- >>>> > testenv/Test-O.py | 3 +- >>>> > testenv/Test-Parallel-Proto.py | 6 +- >>>> > testenv/Test-Post.py | 3 +- >>>> > testenv/Test-Proto.py | 6 +- >>>> > testenv/Test-auth-basic-fail.py | 3 +- >>>> > testenv/Test-auth-basic.py | 3 +- >>>> > testenv/Test-auth-both.py | 3 +- >>>> > testenv/Test-auth-digest.py | 3 +- >>>> > testenv/Test-auth-no-challenge-url.py | 3 +- >>>> > testenv/Test-auth-no-challenge.py | 3 +- >>>> > testenv/Test-auth-retcode.py | 3 +- >>>> > testenv/Test-auth-with-content-disposition.py | 3 +- >>>> > testenv/Test-c-full.py | 3 +- >>>> > testenv/Test-cookie-401.py | 3 +- >>>> > testenv/Test-cookie-domain-mismatch.py | 3 +- >>>> > testenv/Test-cookie-expires.py | 3 +- >>>> > testenv/Test-cookie.py | 3 +- >>>> > testenv/WgetTest.py | 337 >>>> > ------------------- >>>> > testenv/conf/__init__.py | 49 +++ >>>> > testenv/conf/authentication.py | 9 + >>>> > testenv/conf/expect_header.py | 7 + >>>> > testenv/conf/expected_files.py | 42 +++ >>>> > testenv/conf/expected_ret_code.py | 16 + >>>> > testenv/conf/files_crawled.py | 18 + >>>> > testenv/conf/hook_sample.py | 15 + >>>> > testenv/conf/local_files.py | 12 + >>>> > testenv/conf/reject_header.py | 7 + >>>> > testenv/conf/response.py | 7 + >>>> > testenv/conf/rule_sample.py | 10 + >>>> > testenv/conf/send_header.py | 7 + >>>> > testenv/conf/server_conf.py | 11 + >>>> > testenv/conf/server_files.py | 15 + >>>> > testenv/conf/urls.py | 10 + >>>> > testenv/conf/wget_commands.py | 10 + >>>> > testenv/exc/__init__.py | 0 >>>> > testenv/exc/test_failed.py | 7 + >>>> > testenv/misc/__init__.py | 0 >>>> > testenv/misc/colour_terminal.py | 29 ++ >>>> > testenv/misc/constants.py | 3 + >>>> > testenv/misc/wget_file.py | 16 + >>>> > testenv/server/__init__.py | 0 >>>> > testenv/server/ftp/__init__.py | 0 >>>> > testenv/server/ftp/ftp_server.py | 162 +++++++++ >>>> > testenv/server/http/__init__.py | 0 >>>> > testenv/server/http/http_server.py | 467 >>>> > ++++++++++++++++++++++++++ >>>> > testenv/test/__init__.py | 0 >>>> > testenv/test/base_test.py | 223 ++++++++++++ >>>> > testenv/test/http_test.py | 45 +++ >>>> > 58 files changed, 1451 insertions(+), 1035 deletions(-) >>>> > delete mode 100644 testenv/ColourTerm.py >>>> > delete mode 100644 testenv/FTPServer.py >>>> > delete mode 100644 testenv/HTTPServer.py >>>> > delete mode 100644 testenv/WgetTest.py >>>> > create mode 100644 testenv/conf/__init__.py >>>> > create mode 100644 testenv/conf/authentication.py >>>> > create mode 100644 testenv/conf/expect_header.py >>>> > create mode 100644 testenv/conf/expected_files.py >>>> > create mode 100644 testenv/conf/expected_ret_code.py >>>> > create mode 100644 testenv/conf/files_crawled.py >>>> > create mode 100644 testenv/conf/hook_sample.py >>>> > create mode 100644 testenv/conf/local_files.py >>>> > create mode 100644 testenv/conf/reject_header.py >>>> > create mode 100644 testenv/conf/response.py >>>> > create mode 100644 testenv/conf/rule_sample.py >>>> > create mode 100644 testenv/conf/send_header.py >>>> > create mode 100644 testenv/conf/server_conf.py >>>> > create mode 100644 testenv/conf/server_files.py >>>> > create mode 100644 testenv/conf/urls.py >>>> > create mode 100644 testenv/conf/wget_commands.py >>>> > create mode 100644 testenv/exc/__init__.py >>>> > create mode 100644 testenv/exc/test_failed.py >>>> > create mode 100644 testenv/misc/__init__.py >>>> > create mode 100644 testenv/misc/colour_terminal.py >>>> > create mode 100644 testenv/misc/constants.py >>>> > create mode 100644 testenv/misc/wget_file.py >>>> > create mode 100644 testenv/server/__init__.py >>>> > create mode 100644 testenv/server/ftp/__init__.py >>>> > create mode 100644 testenv/server/ftp/ftp_server.py >>>> > create mode 100644 testenv/server/http/__init__.py >>>> > create mode 100644 testenv/server/http/http_server.py >>>> > create mode 100644 testenv/test/__init__.py >>>> > create mode 100644 testenv/test/base_test.py >>>> > create mode 100644 testenv/test/http_test.py >>>> > >>>> > -- >>>> > 1.8.3.2 >>>> > >>>> > 2014-03-14 21:00 GMT+08:00 Darshit Shah <[email protected]>: >>>> > >>>> > On Fri, Mar 14, 2014 at 1:20 PM, 陈子杭 (Zihang Chen) >>>> > <[email protected]> >>>> >> wrote: >>>> >> > >>>> >> > >>>> >> > >>>> >> > 2014-03-14 20:01 GMT+08:00 Darshit Shah <[email protected]>: >>>> >> >> >>>> >> >> Hi, >>>> >> >> >>>> >> >> Things are getting much cleaner with each iteration. :) >>>> >> >> I haven't had the time to check the patchset yet. And I may not >>>> >> >> have >>>> >> the >>>> >> >> time until Monday either. However, I do have a few high level >>>> >> >> comments: >>>> >> >> >>>> >> >> 1. Please upload all patches directly to the mail. Do not compress >>>> >> >> them. It may look ugly, but it is much more convenient. >>>> >> >> This is fixed, if you follow Yousong's recommendations on how to >>>> >> >> send >>>> >> >> patches. Using git for it all is always a good idea. >>>> >> >> >>>> >> > Yes, I'll follow this advice. >>>> >> >> >>>> >> >> 2. Your commit messages are still too long. Always follow 80 chars >>>> >> >> per line. If you wish to add more, the correct way is to write a >>>> >> >> short >>>> >> 80 >>>> >> >> char message, leave a blank line and then write however long a >>>> >> >> commit >>>> >> >> message you want. >>>> >> >> The point is, the first line goes into the repository, patch, >>>> >> >> name, >>>> >> etc. >>>> >> >> It >>>> >> >> is supposed to be an extremely short description of the commit. >>>> >> >> Later, >>>> >> >> in the body, you can expand as much as you want. Again, I'm not >>>> >> >> sure >>>> >> >> what your editor is, but with the git syntax files, ViM highlights >>>> >> >> and >>>> >> >> lets >>>> >> >> you know when you've exceeded the maximum line length or are >>>> >> >> writing on the 2nd line of a commit message, which generally >>>> >> >> should be >>>> >> >> empty. >>>> >> > >>>> >> > I edit ChangeLog, README with vim, if you're talking about them. I >>>> >> checked >>>> >> > them with Kate, it surely does exceed 2 or 3 chars in some lines. >>>> >> > Wired. >>>> >> > I'll try fixing my .vimrc. >>>> >> Yes. The same. Vim's textwidth is complex. You probably have a >>>> >> wrapmargin >>>> >> of 2. And that is fine. Your commit messages had multiple sentences, >>>> >> which >>>> >> is hard to read. >>>> >> >>>> >> Also, check http://blog.ezyang.com/2010/03/vim-textwidth/ for a >>>> >> better >>>> >> understanding of the textwidth features. >>>> >> >>>> >> >> >>>> >> >> >>>> >> >> 3. Also, I'm not a fan of making things very Pythonic. It, in my >>>> >> personal >>>> >> >> opinion makes code more unreadable for the general non-Python >>>> >> >> developer. The whole point of using Python is to ensure that a >>>> >> >> random >>>> >> >> C developer can read and follow the code. Hence, I was very >>>> >> >> particular >>>> >> >> about not using fancy Python features in the Test Suite. The >>>> >> >> module >>>> >> >> was horrible, I concede (I'm no Object oriented programmer, I like >>>> >> >> C), >>>> >> >> but I would prefer the code remains simpler. I'll give more >>>> >> >> specific >>>> >> >> examples and pointers to locations which I think should be >>>> >> >> improved >>>> >> >> once I look at the code. >>>> >> > >>>> >> > I totally understand what you're talking about and I agree with >>>> >> > you. >>>> >> > However, >>>> >> > I still insist that under certain circumstances, it's better to be >>>> >> > a >>>> >> little >>>> >> > more >>>> >> > Pythonic. IMO, C developers who uses this test suite rarely need to >>>> >> modify >>>> >> > the framework of the test suite. They only need to write new test >>>> >> > case >>>> >> > scripts, and if needed, write new hook or rule. They can write >>>> >> > however >>>> >> they >>>> >> > like. I just need make the interface as plain as possible. >>>> >> >> >>>> >> Sure. Being pythonic is not always bad. Also, I agree that most of >>>> >> the >>>> >> times >>>> >> you do not need to edit the core framework. Especially as long as you >>>> >> provide >>>> >> and excellent API. However, oftentimes, you want to know what's going >>>> >> on >>>> >> in the Test Suite setup, just to see what can be tweaked and how. >>>> >> Else, >>>> >> you >>>> >> simply want to know how it runs, and you try to follow the code. >>>> >> >>>> >> Specifically, I'd like to stay away from concepts like aliases which >>>> >> can >>>> >> get >>>> >> confusing sometimes for someone not used to Python. >>>> >> >>>> >> >> >>>> >> >> On Fri, Mar 14, 2014 at 10:28 AM, 陈子杭 (Zihang Chen) < >>>> >> [email protected]> >>>> >> >> wrote: >>>> >> >> > Thank you very much for your advice! >>>> >> >> > >>>> >> >> > >>>> >> >> > 2014-03-14 16:58 GMT+08:00 Yousong Zhou <[email protected]>: >>>> >> >> > >>>> >> >> >> Hi, Zihang. >>>> >> >> >> >>>> >> >> >> You are making progress. :) Below are my comments and hope >>>> >> >> >> that >>>> >> will >>>> >> >> >> help you in some way. >>>> >> >> >> >>>> >> >> >> - I haven't got the chance to try python3, but in python2, >>>> >> >> >> there is >>>> >> >> >> the difference between traditional classes and new-style >>>> >> >> >> classes, >>>> >> i.e. >>>> >> >> >> "class C:" versus "class C(obj):". If it is still there in >>>> >> >> >> python3, >>>> >> >> >> new-style classes is generally preferred. >>>> >> >> >> >>>> >> >> > Yes, there are only new-style classes in Python3. >>>> >> >> > >>>> >> >> >> - In the newly introduced "conf" package, if it is your >>>> >> >> >> intention >>>> >> >> >> that "gen_hook()" is not expected to be exported, then you may >>>> >> >> >> want >>>> >> to >>>> >> >> >> enforce it in some way. >>>> >> >> >> >>>> >> >> > Sure, I'll see what I can do. >>>> >> >> > >>>> >> >> >> - "Refactor a lot" is truly a lot for a single commit. :) >>>> >> >> >> - I randomly check on the 8th patch. >>>> >> >> >> >>>> >> >> >> 1. Why there are three names of almost the same meaning >>>> >> appearing >>>> >> >> >> in the code, i.e. ExpectedRetcode, ExpectedRetCode, >>>> >> ExpectedReturnCode >>>> >> >> >> (This one was found in the repository). >>>> >> >> >> >>>> >> >> > The first one is the one I think is named inappropriately >>>> >> >> > (Retcode => >>>> >> >> > RetCode). >>>> >> >> > But to maintain compatibility (ExpectedRetcode is still used in >>>> >> >> > test >>>> >> >> > case >>>> >> >> > scripts), >>>> >> >> > it's still in use utilizing register(alias='ExpectedRetcode'). >>>> >> >> > >>>> >> >> >> 2. The newly formatted error string will not run, since >>>> >> >> >> "expected_ret_code" is a int, and it will not format with "%s". >>>> >> >> >> >>>> >> >> > Actually, an int object will be converted to str automatically >>>> >> >> > using >>>> >> >> > __str__ if >>>> >> >> > formatted with '%s'. (I use format int with '%s' all the time.) >>>> >> >> > >>>> >> >> >> 3. I vaguely remembered that a new format string syntax was >>>> >> >> >> proposed and recommended. While at it, is it possible that >>>> >> >> >> this >>>> >> >> >> feature be used? >>>> >> >> >> >>>> >> >> > Yes, there is one. However using this feature will cause changes >>>> >> >> > in >>>> >> the >>>> >> >> > test >>>> >> >> > case scripts, because in the test cases {{port}} is used, while >>>> >> >> > the >>>> >> >> > format >>>> >> >> > syntax >>>> >> >> > is {port}. I'll think about it though. >>>> >> >> > >>>> >> >> >> 4. If the decorator "register()" is about "conf", then >>>> >> >> >> "conf" or >>>> >> >> >> "register_conf" should be a better name, for the same reason it >>>> >> >> >> is >>>> >> >> >> "staticmethod" instead of "register_staticmethod". >>>> >> >> >> >>>> >> >> > Yes! Didn't think about it. I think I'll rename register to >>>> >> >> > "rule" >>>> >> and >>>> >> >> > "hook" for >>>> >> >> > different type of confs, though rule and hook are the same >>>> >> >> > thing. >>>> >> >> > >>>> >> >> >> 5. On the expected_files.py, files_crawled.py, etc. you may >>>> >> >> >> want >>>> >> >> >> to format a proper error string and raise an exception with it >>>> >> instead >>>> >> >> >> of "print" it to stdout. >>>> >> >> >> >>>> >> >> > Yes, I'll think about it. >>>> >> >> > >>>> >> >> >> >>>> >> >> >> - Double blank lines. >>>> >> >> >> >>>> >> >> > Ouch, I thought methods are separated with two blank lines in >>>> >> >> > PEP8. >>>> >> >> > Looks like I'm wrong. I'll try fix this. >>>> >> >> > >>>> >> >> >> - Notice the "No newline at end of file" messages generated by >>>> >> >> >> git. >>>> >> >> >> Vim will automatically add a newline at end of a file, not sure >>>> >> what's >>>> >> >> >> your editor for this. >>>> >> >> > >>>> >> >> > Yes, I'll fix my editor for this. (I actually in my latest >>>> >> >> > patches >>>> >> >> > delete >>>> >> >> > the >>>> >> >> > newline at end of files on purpose. Looks like I got it wrong >>>> >> >> > again.) >>>> >> >> > >>>> >> >> >> >>>> >> >> >> >>>> >> >> > Below are my personal preferences on SubmittingPatches, pick >>>> >> >> > some if >>>> >> >> >> they seem to be reasonable. >>>> >> >> >> >>>> >> >> >> - Describe your overall plan and intention in a cover letter >>>> >> >> >> can be >>>> >> >> >> good. Try it with --cover-letter option when generating >>>> >> >> >> patches >>>> >> with >>>> >> >> >> "git format-patch". >>>> >> >> >> - Cover letter can also be used to track differences between >>>> >> versions >>>> >> >> >> of your patch series so that reviewers can easily know what's >>>> >> >> >> new in >>>> >> >> >> this version. Try it with --subject-prefix='PATCH vN' option >>>> >> >> >> when >>>> >> >> >> using "git format-patch". >>>> >> >> >> - The Subject line of your should be short and concise, fitted >>>> >> >> >> into >>>> >> >> >> one line sentence without commas, and detailed description goes >>>> >> >> >> to >>>> >> >> >> message body. >>>> >> >> >> - I prefer patches sent with "git send-email 000*.patch", that >>>> >> >> >> way >>>> >> we >>>> >> >> >> can view and refer to the specific lines of the patch with >>>> >> >> >> context. >>>> >> >> >> >>>> >> >> >> Thanks very much for these suggestions! I'll see what I can do. >>>> >> >> > >>>> >> >> > >>>> >> >> >> Link [1] may serve as a good example. I can only find guide >>>> >> >> >> for >>>> >> >> >> submitting patches to wget with Mercurial [2]. >>>> >> >> >> >>>> >> >> >> [1] >>>> >> >> >> >>>> >> >> >> >>>> >> >>>> >> http://www.linux-mips.org/archives/linux-mips/2011-01/threads.html#00006 >>>> >> >> >> [2] >>>> >> >> >> >>>> >> >> >> >>>> >> >>>> >> http://wget.addictivecode.org/PatchGuidelines#How_to_create_patches_with_Mercurial >>>> >> >> >> >>>> >> >> >> >>>> >> >> >> yousong >>>> >> >> >> >>>> >> >> >> On 14 March 2014 10:16, 陈子杭 (Zihang Chen) <[email protected]> >>>> >> wrote: >>>> >> >> >> > Hi. >>>> >> >> >> > >>>> >> >> >> > Just finished on my latest patches. >>>> >> >> >> > >>>> >> >> >> > >>>> >> >> >> > 2014-03-12 22:49 GMT+08:00 Darshit Shah <[email protected]>: >>>> >> >> >> > >>>> >> >> >> >> Hi, >>>> >> >> >> >> >>>> >> >> >> >> That's great! Thanks for the patches. I do have a few >>>> >> >> >> >> comments >>>> >> about >>>> >> >> >> >> it >>>> >> >> >> >> though: >>>> >> >> >> >> >>>> >> >> >> >> 1. You are still missing ChangeLog entries for each patch. >>>> >> >> >> >> You >>>> >> >> >> >> should >>>> >> >> >> >> Ideally have a ChangeLog entry for every commit. >>>> >> >> >> >> 2. I am not sure I completely follow the logic of the extra >>>> >> lines of >>>> >> >> >> >> code that you've added to colour_terminal.py >>>> >> >> >> >> 3. More importantly, you moved the module ColourTerm, but >>>> >> >> >> >> did not >>>> >> >> >> >> change the import statements in any file. >>>> >> >> >> >> Each commit should build successfully. That is why we have >>>> >> smaller >>>> >> >> >> >> commits. A failure in this commit will give >>>> >> >> >> >> false positives when someone attempts to use git bisect to >>>> >> >> >> >> find a >>>> >> >> >> >> regression. >>>> >> >> >> >> 4. Your commit message states, "move ColourTerm.py to >>>> >> >> >> >> misc/colour_terminal.py" but you're doing more than that. >>>> >> >> >> >> >>>> >> >> >> >> I would suggest you move all the files you want in one >>>> >> >> >> >> commit and >>>> >> >> >> >> then >>>> >> >> >> >> edit them later in different commits. >>>> >> >> >> >> Please fix the patches so that every commit compiles and >>>> >> >> >> >> runs the >>>> >> >> >> >> tests independently. A commit should not >>>> >> >> >> >> depend on patches that come in the following commit. >>>> >> >> >> >> >>>> >> >> >> >> >>>> >> >> >> >> On Wed, Mar 12, 2014 at 3:14 PM, 陈子杭 (Zihang Chen) >>>> >> >> >> >> <[email protected]> >>>> >> >> >> >> wrote: >>>> >> >> >> >> > 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 >>>> >> >> >> >> > --- >>>> >> >> >> >> > 此致 >>>> >> >> >> >> > 陈子杭 >>>> >> >> >> >> > 武汉大学计算机学院 >>>> >> >> >> >> > >>>> >> >> >> >> >>>> >> >> >> >> >>>> >> >> >> >> >>>> >> >> >> >> -- >>>> >> >> >> >> 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 >>>> >> > --- >>>> >> > 此致 >>>> >> > 陈子杭 >>>> >> > 武汉大学计算机学院 >>>> >> > >>>> >> >>>> >> >>>> >> >>>> >> -- >>>> >> 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 >> --- >> 此致 >> 陈子杭 >> 武汉大学计算机学院 >> > > > > -- > Regards, > Chen Zihang, > Computer School of Wuhan University > --- > 此致 > 陈子杭 > 武汉大学计算机学院 > -- Thanking You, Darshit Shah
