On Fri, 2008-06-13 at 00:43 +0200, Javier Martín wrote: > El jue, 12-06-2008 a las 16:31 -0400, Pavel Roskin escribió: > > Please don't add any trailing whitespace. Line in the patch that start > > with a plus should not end with a space or a tab. > I could not find the line you were referring to. I could only find one > line starting with a plus (in install_int13_handler), but it does not > end with a space/tab. However, I did find one trailing tab after a > comment, and removed it.
I mean the lines your patch adds. > > Please avoid camelCase names, such as bpaMemInKb and retVal. Local > > variables should generally be short, like "ret" and "bpa_mem". > > > > Some strings are excessively long. While there may be exception of the > > 80 column limit, I see a 118 character long line that can be trivially > > wrapped. > Both corrected. retVal is still there. Please call it "ret", it's the traditional name of the variable to be returned. > Sorry, I work on a wide screen and, tough I tried to > uphold the 80-char line rule, I'm not used to it. I think there was a > switch in gedit to graphically display the 80-char limit, but I can't > find it now... Edit->Preferences->View->Right Margin > OOC: do you know if there is a syntax highlighting mode for gas/x86 > assembly code in gedit? Right now the program thinks the code is C, and > the only opcode it highlights is "int". Even a gas-only highlighting > (i.e. only the directives, not the opcodes) would be useful. There are some possibly useful attachments here: http://bugzilla.gnome.org/show_bug.cgi?id=152267 I hope that others will comment on the patch contents. -- Regards, Pavel Roskin _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
