|
Rockbox mail archiveSubject: Re: jdgordon: r12227 - in trunk/apps: . menus recorderRe: jdgordon: r12227 - in trunk/apps: . menus recorder
From: Jonathan Gordon <jdgordy_at_gmail.com>
Date: Thu, 8 Feb 2007 19:42:38 +1100 On 08/02/07, Daniel Stenberg <daniel_at_rockbox.org> wrote: > On Thu, 8 Feb 2007, mailer_at_svn.rockbox.org wrote: > > To me, this move is a wrong move. This introduces a complicated system that > fewer people will understand/change and that will be bad for us in the long > run. > > Some nits in the actual code follows: > > > /* ----------------------------------------------------------------- > > * vim: et sw=4 ts=8 sts=4 tw=78 > > */ > > I realize you didn't add this chunk, but I personally think we should not have > stuff like this in source files. > > > + if (m->flags&MENU_HAS_DESC) > > + menu_callback= m->callback_and_desc->menu_callback; > > + else menu_callback = m->menu_callback; > > This latest line is normally written on two separate lines in other Rockbox > code, and I would appriciate if we stayed with that source code style. You use > this same style in multiple places. > > > + DEBUGF("%x\n",setting->flags); > > + if (setting->flags&F_INT_SETTING) > > + {DEBUGF("boo"); > > These DEBUGF() calls seem like leftovers that should be removed. > forgot about them... I've already seen them and removed them. > > +#ifdef CONFIG_TUNER > > +MENUITEM_FUNCTION(load_radio_screen, ID2P(LANG_FM_RADIO), > > + (menu_function)radio_screen, dynamicitem_callback); > > +#endif > > + > > +#include "settings_menu.h" > > ... here you #include a header in the "middle" of the C file. Why? > Isnt there a comment with them saying its temporary? > > +struct choice_setting { > > + void (*option_callback)(int); > > + int count; > > + unsigned char **desc; > > +}; > > And this is suddenly no longer following the 4-space indent tradition. > Not sure what happened there... ill fix it > -- > Daniel Stenberg -- http://www.rockbox.org/ -- http://daniel.haxx.se/ > Received on 2007-02-08 Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy |