[114457] trunk/dports/python/py-setuptools/Portfile

[Ryan Schmidt]

On Dec 9, 2013, at 00:46, jmr at macports.org wrote:

> Revision
> 114457
> Author
> jmr at macports.org
> Date
> 2013-12-08 22:46:47 -0800 (Sun, 08 Dec 2013)
> Log Message
> 
> py-setuptools: update to 2.0 for python 2.6+
> Modified Paths
> 
> 	• trunk/dports/python/py-setuptools/Portfile


> -if {${name} ne ${subport}} {
> +if {$subport ne $name} {

The previous change to this line was mine in r114324. My commit message said “use eq and ne when comparing ${subport} instead of == and !=” but in fact in this port (and others) I made three changes:

1. changed ==/!= to eq/ne
2. put curly braces around the variable names
3. flipped the order of the variables

I assume because you reverted two of those changes that you disagree with them. I know we should refrain from making stylistic changes to other people’s ports without asking them first, and I assume the fact that I made them to your port anyway annoyed you, and I’m sorry about that. Let me explain why I made these changes.

Part of the de facto MacPorts programming style, in portfiles anyway, has been to always use curly braces around variable names. As you know that’s not required in all cases, but it increases legibility and decreases cognitive load to always use the same variable syntax. I consider it a best practice. In many of the python ports for some reason this had not been done in the line that checks name against subport, though it is being done in most other lines, such as when you use ${version}:

> +    if {${python.version} <= 25} {
> +        version     1.4.2
> +        distname    setuptools-${version}

So when I went through all the python ports a few days ago to switch the string comparisons, since I was already touching that line, I took the opportunity to also add the curly braces, both for consistency with other ports and for consistency within each port.

I also wanted to make the order of the variables consistent, and I happened to choose name first, subport second, because that’s the order I’ve used in my ports.


In a later commit, when I adjusted the string comparisons for e.g. ${os.platform}, in some cases I flipped the order as well. In many cases I had previously written e.g.

if {“darwin” == ${os.platform}}

I started writing comparisons this way in portfiles based on a habit I had from other programming languages wherein, when comparing a variable to a value, you write the value first and the variable second (“if (5 == x)”), rather than the other way around (“if (x == 5)”), to avoid accidentally assigning the value to the variable if you inadvertently omit one of the equals signs (“if (x = 5)”). But since in Tcl the syntax for assigning a variable is different from the syntax for accessing a variable there was never any danger of this anyway so I wanted to begin removing this mental error from portfiles too.


I thought about filing tickets for these changes beforehand, but it would have been more work and I didn’t think there would be any objection. And I feared that filing a ticket and waiting a few days to give everyone time to read it would have led to some of the ports being updated for other reasons in the mean time, possibly leading to conflicts with my changes and then more work to resolve them.


So in summary, I apologize for making stylistic changes without prior consultation; I like the changes I made and I think we should keep them; but if we don’t want to keep them that’s ok too, I just wanted to make my reasons known.