[Dextrose] NamingAlert being optional

[Martin Abente]

On Wed, Nov 3, 2010 at 4:09 PM, Anish Mangal <anishmangal2002 at gmail.com>wrote:

> Hi,
>
> My two cents on this...
>
> 0001-S-non-intrusive-NamingAlert.patch
>
> +        <long>If TRUE NamingAlert will show when
> +        the activity close.</long>
>
> s/If TRUE/If TRUE,/  (comma)
> s/show/popup/
> s/close/closes/
>
> ***
>
> 0001-STK-non-intrusive-NamingAlert.patch
>
> -        if skip_save or self._jobject is None or \
> +        client = gconf.client_get_default()
> +        show_alert = client.get_bool('/desktop/sugar/show_naming_alert')
>
> /me being nitpicky, but perhaps show_naming_alert would be a better
> variable name as compared so show_alert
>

you are right, but i thought it was a little too long and did not want to
break
the line just because of that. I think the identifier self explains enough
in this
case :).


>
> Also, don't know if the commit title message could be better. "Add
> show_naming_alert gconf key" or something like that instead of "S
> non-intrusive NamingAlert"
>
>
Right again, i should mention the key name at least on the comments.

Thanks a lot for the review!



> On Wed, Nov 3, 2010 at 4:06 AM, Martin Abente
> <martin.abente.lahaye at gmail.com> wrote:
> > Hello amigos,
> >
> > I keep following the dextrose TODO list [1]. This time I made simple
> patches
> > to make NamingAlert (the details popup that appears when the activity
> > closes) optional.
> > These patches should cleanly apply to Dextrose code base.
> >
> > 0001-S-non-intrusive-NamingAlert.patch applies to sugar.
> > 0001-STK-non-intrusive-NamingAlert.patch applies to sugar-toolkit
> >
> > This is a review request :)
> >
> > 1 .http://wiki.sugarlabs.org/go/Dextrose/TODO
> >
> > _______________________________________________
> > Dextrose mailing list
> > Dextrose at lists.sugarlabs.org
> > http://lists.sugarlabs.org/listinfo/dextrose
> >
> >
>
> Cheers,
> Anish
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20101103/1d6e5aa5/attachment-0001.html>