Disclaimer: The following is my own opinion and I am not the only who can apply the patches.
-=| Cristian Ionescu-Idbohrn, Wed, Apr 14, 2010 at 12:39:21AM +0200 |=- > On Tue, 13 Apr 2010, Santi Béjar wrote: > > Send it in-line so we can easily comment your changes. > > Sure, I could do that and then find out others prefer attachments, because > their mail servers/clients mangle the contents (wrap lines and damage > whitespace). What do the maintainers prefer? Ideally as an attachment, using 'inline' content-disposition. Having the patches extracted using 'git format-patch' is preferred as this would include the commit messages and authorship. > > Provide a changelog (and changes to debian/changelog). I see it is > > just a proposal, but still it is better if you explain why this > > changes are good (even if you already wrote them in another mail) I'd say that changelog entries aren't obligatory as good commit messages can be used automaticaly to fill those. A note about comments and verification of function arguments. These are kept at minimum because some of the scripts are used in each ACPI event and need to be as quick as possible. Consider pressing and holding down a volume key. Events come several per second. According to the comments in the default file, dash is significantly slowed down by comments. Maybe we can ship the scripts without comments in the binary package, as is done with the default file. That would make hacking of the installed scripts harder though :/ > Another observation is the 'notify' function seems to require > 3 arguments, but never validates that. Never rely user input is > correct. There is no user input. The only "user" are the other scripts in the package. Is some of them not supplying three arguments? Of course, validation can still be made, but I think performance will suffer. And it works now, doesn't it? > > - if [ "x$ENABLE_OSD" = "xno" ]; then > > - return > > - fi > > + [ "$ENABLE_OSD" != no ] || return > > > > I don't like so many negative logic, I am used to that from makefiles, where it is inconvenient to split commands on multiple lines, and commands are required to return true. This is shell, though, so the readability of a multi-line if is something I value more. > Why? All modern shells do the right thing without that ancient 'x'. > That's bloat. And why do you need to quote something (non-null) that is > not going to expand to anything different than it already is? In other > words, isn't this readable and less bloaty? It is. Trimming your nails also helps you lose weight :) > > - commands \ > > - | su "$user" -c "$GOSDC -s --dbus" > > + commands | > > + su "$user" -c "$GOSDC -s --dbus" > > > > Why did you change the continuation sequence? > > Because shell is an iterpreted language. > Because parsing shell commands in real time is expensive. This: > > foo \ > | bar > > is subtilely more expensive than this: > > foo | > bar I'd like to see some numbers backing this claim. I find the former a bit more readable, but otherwise consider it a mater of taste. Personaly, I am much happier applying patches if they address problems somebody is experiencing. Is the code so "bloated" and "ugly" as to prevent contribution?
signature.asc
Description: Digital signature
_______________________________________________ Debian-eeepc-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/debian-eeepc-devel
