Message214138
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. |
|
Date |
User |
Action |
Args |
2014-03-19 21:20:36 | zach.ware | set | recipients:
+ zach.ware, terry.reedy, jkloth, ezio.melotti, r.david.murray, michael.kearney, kathweaver |
2014-03-19 21:20:36 | zach.ware | set | messageid: <1395264036.89.0.0427500585465.issue17846@psf.upfronthosting.co.za> |
2014-03-19 21:20:36 | zach.ware | link | issue17846 messages |
2014-03-19 21:20:36 | zach.ware | create | |
|