Zachary Ware added the comment: Review of kw-issue17846.diff:
setup.rst - hunks 1 & 2 (@@ -49,9 and @@ -216,7): Both changes should be reverted. The change from 3.4 -> 3.3 reverts a very recent change made due to 3.4 becoming the newest maintenance branch. If I'm not mistaken, the blank line after the Windows heading is important. - hunk 3 (@@ -240,11) I think the added sentence here would be better tacked onto the end of the previous paragraph: "[limitations are] here (2010). Not listed on those pages, Visual C++ Express does not support Solution Folders. Because the Python solution file uses Solution Folders, VS Express will warn you about that fact every time you open the ``pcbuild.sln`` file. You can safely dismiss the warning with no impact on your ability to build Python." If you can improve on my English or brevity there, please do :) Also, there's a lot of trailing whitespace in that paragraph. I'd suggest using an editor that can strip trailing spaces for you (I like Notepad++). setupwindows.rst - throughout Lots of trailing whitespace. - line 7 Seems a bit pointless to point back and forth between the two sections. I'm open to suggestions on how best to avoid that. - line 11 A suggestion of TortoiseHg and a link thereto wouldn't be amiss. - line 12 Source for what? What download page? - line 13-14 Doesn't fit here in the narrative flow. This should be in the section about Subversion. - line 17-19 Subversion and Perl should be capitalized. Also, this isn't true: svn is necessary to pull the external library sources from svn.python.org, but you can actually download your sources directly from their respective vendors, provided you put them where the VS project files expect them to be. Perl is not necessary at all if you pulled your OpenSSL sources from svn.python.org, it is only required if you're using sources you got straight from the OpenSSL folks. - line 21-30 This should be rewritten to explain the problem, then suggest a solution. Something like: """ The VS project files expect external libraries that they need to be in folders on the same level as the cpython source checkout, for example, at ``PCbuild\..\..\openssl-1.0.1e``. Since different versions of Python use different versions (or different builds) of the various external libraries, it is recommended to build different Python versions in isolated folders. For example: C:\ |_ Source |_ python3.4 | |_ cpython | |_ tcl-8.6.1.0 | |_ tk-8.6.1.0 | |_ tcltk | |_ ... | |_ python3.3 |_ cpython |_ tcl-8.5.11.0 |_ tk-8.5.11.0 |_ tcltk |_ ... """ Either way, the term "buildbots" is used incorrectly here, referring to the scripts in the Tools/buildbot directory, which our CI system ("the buildbots", see buildbot.python.org) use to build and test Python. - line 36 Subversion should be capitalized. This would also be the place to explain why svn is needed, and also to link to a place to get it. TortoiseSVN is probably a good choice to link to, and mention its installer option to add svn to PATH. - line 42 Perl should be capitalized. It should be explained that Perl is not absolutely necessary (and the cases in which it is). It would also be good to link to ActivePerl, which is what the build_ssl.py script actually looks for. - line 48-50 Grammar issues. Also, this is covered in the _windows-compiling section. - line 52 This is not true. - line 58 There should be an explanation of why. - line 64 Incorrect use of the term "buildbots". - line 70 Those scripts download the sources for every external project, but only build Tcl and Tk. - line 71 Sentence fragment. Unfinished thought? - line 78-93 Poor grammar. Also, none of this is true: I have never had any problems letting the solution build OpenSSL. There is no need to jump through those hoops. If building with pcbuild.sln doesn't work correctly, it's a bug that should be fixed, not documented as being broken. - line 96 Incorrect use of the term "buildbot". - line 125-126 I would rephrase this as "`Python Tools for Visual Studio <...>`_ is an add-on published by Microsoft for editing Python in Visual Studio. It is not usable with Express editions." Frankly, I don't see much added benefit from the new file. It's mostly duplication of PCbuild/readme.txt, and what little isn't already there should be. I would much rather see a patch making sure PCbuild/readme.txt is up to date, and a link to http://hg.python.org/cpython/file/default/PCbuild/readme.txt added to the devguide's existing compilation on Windows section. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue17846> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com