Did I miss the decision to untabify all of the C code?
It looks like we're moving ahead with removing tabs. Was there consensus on this? Last I saw Antoine had written a script that might do what we want, but hadn't been thoroughly tested. Now I've seen a few checkins for files that have been run through the script. What gives? And why do this so close to 2.7? I don't think it will cause any problems, but it's hard to review commits to ensure they have no changes when there's a rush of large commits near a release. Eric.
2010/5/5 Eric Smith <eric@trueblade.com>:
It looks like we're moving ahead with removing tabs. Was there consensus on this?
Last I saw Antoine had written a script that might do what we want, but hadn't been thoroughly tested. Now I've seen a few checkins for files that have been run through the script.
What gives? And why do this so close to 2.7? I don't think it will cause any problems, but it's hard to review commits to ensure they have no changes when there's a rush of large commits near a release.
I presume Victor was acting on an older decision that said that files mixed with tabs and spaces can be reindented for consistency. -- Regards, Benjamin
Eric Smith <eric <at> trueblade.com> writes:
Last I saw Antoine had written a script that might do what we want, but hadn't been thoroughly tested. Now I've seen a few checkins for files that have been run through the script.
As far as I'm concerned, it was a case of eating my own dog food: running the script over a couple of files I'm interested in (_ssl.c, _fileio.c). I believe Victor processed posixmodule.c for the same reasons.
What gives? And why do this so close to 2.7? I don't think it will cause any problems, but it's hard to review commits to ensure they have no changes when there's a rush of large commits near a release.
Well, however soon or late we do this, good luck reviewing multi-thousand line commits to check no mistake sneaked in :) By construction, these commits only adjust whitespace in some C files, which means the risk of breakage is very close to zero. (I guess you could do a "svn diff -x -w" between each two revisions to expose any potential non-whitespace changes) Really Antoine.
Antoine Pitrou wrote:
Eric Smith <eric <at> trueblade.com> writes:
Last I saw Antoine had written a script that might do what we want, but hadn't been thoroughly tested. Now I've seen a few checkins for files that have been run through the script.
As far as I'm concerned, it was a case of eating my own dog food: running the script over a couple of files I'm interested in (_ssl.c, _fileio.c). I believe Victor processed posixmodule.c for the same reasons.
What gives? And why do this so close to 2.7? I don't think it will cause any problems, but it's hard to review commits to ensure they have no changes when there's a rush of large commits near a release.
Well, however soon or late we do this, good luck reviewing multi-thousand line commits to check no mistake sneaked in :)
That's my point. Since it's basically unreviewable, is it smart to do it during a beta? I grant you that it's a largely a mechanized change (except for the "a posteriori manual intervention" part), but still. Eric.
On Wed, May 5, 2010 at 9:59 PM, Eric Smith <eric@trueblade.com> wrote:
Antoine Pitrou wrote:
Eric Smith <eric <at> trueblade.com> writes:
Last I saw Antoine had written a script that might do what we want, but hadn't been thoroughly tested. Now I've seen a few checkins for files that have been run through the script.
As far as I'm concerned, it was a case of eating my own dog food: running the script over a couple of files I'm interested in (_ssl.c, _fileio.c). I believe Victor processed posixmodule.c for the same reasons.
What gives? And why do this so close to 2.7? I don't think it will cause any problems, but it's hard to review commits to ensure they have no changes when there's a rush of large commits near a release.
Well, however soon or late we do this, good luck reviewing multi-thousand line commits to check no mistake sneaked in :)
That's my point. Since it's basically unreviewable, is it smart to do it during a beta?
Hello folks - I don't think these modifications are that "unreviewable": the generated binaries have to be exactly the same with the untabified files don't they? So is a matter of stashing the binaries, applying the patches, rebuild and check to see if the binaries match. Any possible script defects undetected by this would be only (C code) indentation, which could be fixed later. Python 2.7 is in beta, but not applying such a fix now would probably mean that python 2.x would forever remain with the mixed tabs, since it would make much less sense for such a change in a minor revision (although I'd favor it even there). js -><-
I grant you that it's a largely a mechanized change (except for the "a posteriori manual intervention" part), but still.
Eric.
On Wed, May 5, 2010 at 8:52 PM, Joao S. O. Bueno <jsbueno@python.org.br> wrote:
Python 2.7 is in beta, but not applying such a fix now would probably mean that python 2.x would forever remain with the mixed tabs, since it would make much less sense for such a change in a minor revision (although I'd favor it even there).
Since 2.7 is likely the last release of the 2.x series, wouldn't it more productive to spend time improving it instead of wasting time on minor details like indentation? -- Alexandre
Le jeudi 06 mai 2010 08:17:14, Alexandre Vassalotti a écrit :
On Wed, May 5, 2010 at 8:52 PM, Joao S. O. Bueno <jsbueno@python.org.br> wrote:
Python 2.7 is in beta, but not applying such a fix now would probably mean that python 2.x would forever remain with the mixed tabs, since it would make much less sense for such a change in a minor revision (although I'd favor it even there).
Since 2.7 is likely the last release of the 2.x series, wouldn't it more productive to spend time improving it instead of wasting time on minor details like indentation?
Because "2.7 is likely the last release of the 2.x series", I think that it will maintained more longer than any other 2.x release. If we change the indentation in 3.x but not in 2.x, it will be harder to port the commits from 3.x to 2.7. I consider that the maintenance is very important for this last release, more important than "improving it". I prefer to work on new features on 3.x and keep 2.7 as stable as we can. -- Victor Stinner http://www.haypocalc.com/
Alexandre Vassalotti <alexandre <at> peadrop.com> writes:
Since 2.7 is likely the last release of the 2.x series, wouldn't it more productive to spend time improving it instead of wasting time on minor details like indentation?
We occasionally waste time on minor details such as code indentation, documentation wording and punctuation, distinguishing "built-in" vs "builtin", etc. :) I don't think it prevents anyone from doing productive work. (besides, my own bug queue for 2.x currently appears to be empty) Then, as pointed out by Victor, if we want to solve the current indentation mixup, we have to do it in all branches so as to avoid making backports more difficult. Regards Antoine.
On Thu, May 6, 2010 at 4:52 AM, Joao S. O. Bueno <jsbueno@python.org.br> wrote:
On Wed, May 5, 2010 at 9:59 PM, Eric Smith <eric@trueblade.com> wrote:
That's my point. Since it's basically unreviewable, is it smart to do it during a beta?
Hello folks - I don't think these modifications are that "unreviewable": the generated binaries have to be exactly the same with the untabified files don't they? So is a matter of stashing the binaries, applying the patches, rebuild and check to see if the binaries match. Any possible script defects undetected by this would be only (C code) indentation, which could be fixed later.
That's not foolproof, though: there are lots of sections of code that will only get compiled on certain platforms, or with certain configure options, etc. Mark
Le jeudi 06 mai 2010 02:59:26, Eric Smith a écrit :
I grant you that it's a largely a mechanized change (except for the "a posteriori manual intervention" part), but still.
Attached patches are the "manual interventation" parts. 99% of the whole patch only changes the indentation. There is just *two* changes not related directly to indentation: - I moved rc and buffer variable (specific to OS/2) at the beginning of the function to avoid { and }. - I added a newline in "static PyObject * os2_error(int code)" The manual editions is mostly to "fix" the indentation. Diff on Python trunk (diff -uw): --------------------------- --- Modules/posixmodule.c.r80843 2010-05-06 10:18:47.000000000 +0200 +++ Modules/posixmodule.c 2010-05-06 10:18:51.000000000 +0200 @@ -470,6 +470,10 @@ { PyObject *d; char **e; +#if defined(PYOS_OS2) + APIRET rc; + char buffer[1024]; /* OS/2 Provides a Documented Max of 1024 Chars */ +#endif d = PyDict_New(); if (d == NULL) return NULL; @@ -505,10 +509,6 @@ Py_DECREF(v); } #if defined(PYOS_OS2) - { - APIRET rc; - char buffer[1024]; /* OS/2 Provides a Documented Max of 1024 Chars */ - rc = DosQueryExtLIBPATH(buffer, BEGIN_LIBPATH); if (rc == NO_ERROR) { /* (not a type, envname is NOT 'BEGIN_LIBPATH') */ PyObject *v = PyString_FromString(buffer); @@ -521,7 +521,6 @@ PyDict_SetItemString(d, "ENDLIBPATH", v); Py_DECREF(v); } - } #endif return d; } @@ -662,7 +661,8 @@ errors are not in a global variable e.g. 'errno' nor are they congruent with posix error numbers. */ -static PyObject * os2_error(int code) +static PyObject * +os2_error(int code) { char text[1024]; PyObject *v; --------------------------- Diff on Python py3k (diff -uw): --------------------------- --- Modules/posixmodule.c.r80845 2010-05-06 10:36:27.000000000 +0200 +++ Modules/posixmodule.c 2010-05-06 10:36:32.000000000 +0200 @@ -445,6 +445,11 @@ #else char **e; #endif +#if defined(PYOS_OS2) + APIRET rc; + char buffer[1024]; /* OS/2 Provides a Documented Max of 1024 Chars */ +#endif + d = PyDict_New(); if (d == NULL) return NULL; @@ -515,10 +520,6 @@ } #endif #if defined(PYOS_OS2) - { - APIRET rc; - char buffer[1024]; /* OS/2 Provides a Documented Max of 1024 Chars */ - rc = DosQueryExtLIBPATH(buffer, BEGIN_LIBPATH); if (rc == NO_ERROR) { /* (not a type, envname is NOT 'BEGIN_LIBPATH') */ PyObject *v = PyBytes_FromString(buffer); @@ -531,7 +532,6 @@ PyDict_SetItemString(d, "ENDLIBPATH", v); Py_DECREF(v); } - } #endif return d; } @@ -672,7 +672,8 @@ errors are not in a global variable e.g. 'errno' nor are they congruent with posix error numbers. */ -static PyObject * os2_error(int code) +static PyObject * +os2_error(int code) { char text[1024]; PyObject *v; --------------------------- -- Victor Stinner http://www.haypocalc.com/
participants (7)
-
Alexandre Vassalotti -
Antoine Pitrou -
Benjamin Peterson -
Eric Smith -
Joao S. O. Bueno -
Mark Dickinson -
Victor Stinner