[aur-dev] AUR Coding recomendations IMPORTANT
Hi, I am writing this e-mail after reviewing the AUR code and some patches that people have been sending. As we know, AUR (like many applications) is still in development, and many of the things that are actually working weren't coded by any of us, and I can understand this. The purpose of my e-mail is to give some recommendations that should be adopted by everyone, I don't want to be arrogant or force you to do something without knowing why, so I will explain the reasons behind my recommendations and then you will probably agree with me. Singles quotes vs Doubles quotes ===================== Problem: In the past I sent an e-mail about this topic, but I think I was misunderstood. The correct way should be using singles quotes when you don't need to parse any kind of variable, that applies in any case. Examples: 1.- include "lib/whatever.inc.php"; // why double quotes ? just to waste resources and no more.. 2.- function myFunction($var) { return "Hello ".$var; // wrong concatenation AND using double quotes ?! # return 'Hello '.$var; // this is the correct use } echo myFunction("world"); // wrong # echo myFunction('world'); // Oh nice you know what i'm talking about. 3.- $_SESSION["myName"] = "MyNameIsWrong"; // and it certainly is wrong, thanks for wasting resources with double quotes! # $_SESSION['myName'] = 'Foo Bar'; // that's the way 4.- // the last example of a block of HTML print "<table>\n"; print "<tr>\n"; print "<td>".$title."</td>\n"; print "</tr>\n"; print "</table>\n"; // Come on!!!!, the following line does the same. echo "<table>\n<tr>\n<td>$title</td>\n</tr>\n</table>"; // Yes I know that placing plenty of \n's would render the code unreadable, but you can do it in one line with a simple echo. Overview: I recommend the use of double quotes in the case that you need to parse many variables so that the code doesn't become unreadable, or if you want to output HTML code in a sorted way. print vs echo ======== Problem: Many of us usually prefer to use print because we are used to the function name similarities from languages other than PHP, and we can't get used to *not* using it. Examples: 1.- print 'echo is ugly!'; // assuming that you've learned about singles and doubles quotes :D Overview: Print outputs the string and has an actual return value of TRUE or FALSE, so that converts print in a function slower than echo. See this thread for more information: http://codingforums.com/archive/index.php?t-41745.html To wrap this up, I just want to emphasize the fact that whereas this might look trivial and nearly irrelevant to some of you, it is important in order to make AUR a better tool. Cheers. -- Angel Velásquez angvp @ irc.freenode.net Linux Counter: #359909 Arch Linux Trusted User
On Fri, Jun 20, 2008 at 07:54:24AM +0200, =?ISO-8859-1?Q?Angel_Vel=E1squez_ wrote:
Hi, I am writing this e-mail after reviewing the AUR code and some patches that people have been sending.
As we know, AUR (like many applications) is still in development, and many of the things that are actually working weren't coded by any of us, and I can understand this.
I think AUR is more in maintenance mode more than development mode. Developers of AUR have been envisioning a better system for awhile but just haven't quite gotten to it. I really just want to get AUR to a cool state where I can devote time to developing an even awesomer system than what currently exists. Hey maybe you could be the next maintainer if you send enough patches. *wink wink*
The purpose of my e-mail is to give some recommendations that should be adopted by everyone, I don't want to be arrogant or force you to do something without knowing why, so I will explain the reasons behind my recommendations and then you will probably agree with me. [...] 4.- // the last example of a block of HTML print "<table>\n"; print "<tr>\n"; print "<td>".$title."</td>\n"; print "</tr>\n"; print "</table>\n"; // Come on!!!!, the following line does the same.
echo "<table>\n<tr>\n<td>$title</td>\n</tr>\n</table>"; // Yes I know that placing plenty of \n's would render the code unreadable, but you can do it in one line with a simple echo.
A much better way to clean this up would be: <table> <tr> <td><?php echo $title; ?></td> </tr> </table> And that's what we're trying to do. Just take as much HTML out of PHP as possible. Thanks for your suggestions. Rock on.
Angel Velásquez writes:
The purpose of my e-mail is to give some recommendations that should be adopted by everyone, I don't want to be arrogant or force you to do something without knowing why, so I will explain the reasons behind my recommendations and then you will probably agree with me.
Singles quotes vs Doubles quotes
Problem:
In the past I sent an e-mail about this topic, but I think I was misunderstood. The correct way should be using singles quotes when you don't need to parse any kind of variable, that applies in any case.
As I feel a little bit addressed by that (read addressed not offended ;-)) I'd like to repeat again that I completely understand your points (and understood them in the prior mails too ;)). I am currently maintaining around 25.000 lines of PHP code (which is not all written by me), and trust me, I know what good coding style is worth (personally I stick to the Zend Framework PHP coding standards [1] whenever possible when it comes to PHP). But, as you already said some source code files of the AUR are not really in shape in many ways when it comes to coding standards. The problem, as I see it for someone who likes to send a patch, is, that you usually address a specific "problem" with a patch. Although cleaning the code makes always sense, it should IMHO be separated into separate patch sets and not mixed with other patches in order to not blur the "real" purpose of a patch. I actually sent one patch which did both, fix some invalid XHTML + refactored the rest of the file. It was rejected because it was too big and you couldn't really tell from the diff what was actually fixed and what was just moved a bit here and there. I think the cleaning of the code is a task of it's own and should not be mixed with fixes for specific problems. Even though the changes I've sent until now where small, I also agree with Loui, when he says the AUR is more in maintenance mode (IIRC there where already two attempts for AUR2, though I don't know much about their current state). Again, I'd just like to say that I believe we're thinking along the same lines :). Best Regards, Michael [1] http://framework.zend.com/manual/en/coding-standard.html
On Fri, Jun 20, 2008 at 11:30:20PM +0200, chi@chimeric.de wrote:
But, as you already said some source code files of the AUR are not really in shape in many ways when it comes to coding standards. The problem, as I see it for someone who likes to send a patch, is, that you usually address a specific "problem" with a patch. Although cleaning the code makes always sense, it should IMHO be separated into separate patch sets and not mixed with other patches in order to not blur the "real" purpose of a patch.
Yep that's the idea. Ooh. I hadn't even thought of this: From: http://framework.zend.com/manual/en/coding-standard.html
For files that contain only PHP code, the closing tag ("?>") is never permitted. It is not required by PHP. Not including it prevents trailing whitespace from being accidentally injected into the output.
That's an easy fix that might prevent some issues from popping up. The rss broke once because of a newline somewhere.
On 21/06/2008, at 5:30 AM, chi@chimeric.de wrote:
Even though the changes I've sent until now where small, I also agree with Loui, when he says the AUR is more in maintenance mode (IIRC there where already two attempts for AUR2, though I don't know much about their current state).
Being the main dev of the second attempt at AUR2, I'd like to know as well :P. I think sticking to coding conventions is a good idea. Developers should be disciplined and strictly adhere to them, it really does make maintenance easier. I'd be great if people also wrote doxygen/phpdoc documentation to make maintenance and later development much, much easier. It doesn't matter if the code won't be used by anyone else, internal documentation is also very useful. Having said that, I don't know if a lot of effort should be put into fixing up the current code base (especially technical writing). I think everyone agrees that the AUR needs a rewrite.
The problem, as I see it for someone who likes to send a patch, is, that you usually address a specific "problem" with a patch. Although cleaning the code makes always sense, it should IMHO be separated into separate patch sets and not mixed with other patches in order to not blur the "real" purpose of a patch.
I don't really see a problem with fixing up the code in terms of conventions along with other changes, as long as only the lines of code that would be changed either way are fixed up. If you go and change a bunch of other code, then that really should be in a separate patch. Of course when cherry-picking patches or something, it sometimes is nicer to have very specific patches, so that it would be possible to apply the "code standard adhering" patch to some other branch.
participants (4)
-
Angel Velásquez
-
chi@chimeric.de
-
Loui
-
Sebastian Nowicki