[Sugar-devel] [GSoC] [Patch] Sugar Smolt Control Panel Integration

[Sebastian Dziallas]

Sorry that it took me a bit to reply. Catching up on email backlog
once again. I dropped a couple of replies inline. :)

On Mon, Jun 28, 2010 at 11:57 AM, Sascha Silbe
<sascha-ml-ui-sugar-devel at silbe.org> wrote:
> Excerpts from Sebastian Dziallas's message of Sun Jun 27 21:07:11 +0200 2010:
>
>> I've been working on an integration of Smolt
>> (https://fedorahosted.org/smolt/) into Sugar as part of my GSoC. The
> Nice!
>
>> The repository lives here
>> (http://git.sugarlabs.org/projects/sugar/repos/sugar-smolt) and
> A quick look doesn't show any major mistakes (take that as praise ;) ).
> There are a few minor style issues; pylint + pep8 might catch some of
> the easy ones (like EOL spaces and naming conventions for constants).

Oh, okay! I'll run that...

> Some questions I had:
> - Why do you recommend to delete the profile (including UUID) after
>  submission? Isn't one of the purposes of smolt support to be able
>  to help individual users with hardware trouble (which would require
>  knowing the UUID of the user)?

It's not the we actually *recommend* to the user to delete the profile
afterwards. It's just that they should have the option to delete it,
if they want to. But yes, indeed: We'd need to know the URL to read
the profile.

> - Is the privacy policy really large enough that we need to destroy the
>  widget even while the section view is active? (If section views are
>  kept in memory even after they got closed, that should be fixed rather
>  than worked around).

Uh, I looked at how the GPL was read in the about-my-computer screen
and that's how they did it there. The privacy policy might be a little
shorter, though.

> Suggestions:
> - Only show the section if smolt is actually installed (not all distros
>  have it). Might need support on the Sugar side as this check is
>  currently hardcoded for the keyboard and power sections. The latter
>  one even checks for /ofw, but that's stuff for the OHM support thread,
>  not this one.

I put that in the __init__.py file: the control panel section only
shows up when the smolt file executing the submission is present.

> - Don't store the handler ids of the GTK callbacks if you don't use them.
>  We don't need to keep a reference in our code to protect them from
>  garbage collection.
> - Maybe deactivate the Delete button if no profile is present? (Submitting
>  a second time can be useful so that button should always be active).

That should be doable, yeah. :)

> - Assuming smoltSendProfile is synchronous (i.e. doesn't finish until
>  sending the profile has either been finished successfully or failed),
>  you should run it in the background. The Sugar shell is currently a
>  single process, so running synchronously will block everything.

Yup yup, I'll look into that!

> - Check for smolt errors (rc, stderr) and relay them to the user.
>
>
>> I marked the ticket as r?, so a review would be appreciated, too.
> Sending the patch to the mailing list makes it easier to review, so you'll
> get more feedback that way.
> My config is rather complicated because of the email address scheme I use,
> but maybe it shows you how to automate everything so sending the patch to
> the ML is as simple as typing a single git command. For patches that might
> need to be revised before they can be committed I use TopGit.
> "git rebase -i" is nice as well, but only works if you don't publish your
> repository.

Thanks for all the feedback and hints, it's really appreciated! :)

--Sebastian

> This is my ~/.gitconfig :
>
> [user]
>        email = sascha-pgp at silbe.org
>        name = Sascha Silbe
> [url "gitorious at git.sugarlabs.org:"]
>        pushInsteadOf = git://git.sugarlabs.org/
> [alias]
>        send-to-ml-multi = "!git send-email -s --annotate --summary --cover-letter --add-header=\"Reply-To: Sascha Silbe <sascha-ml-reply-to-$(python -c 'import time; t=time.gmtime(); print \"%d-%d\" % (t.tm_year, t.tm_mon//4+1)')@silbe.org>\""
>        send-to-ml-single = "!git send-email -s -p --stat --add-header=\"Reply-To: Sascha Silbe <sascha-ml-reply-to-$(python -c 'import time; t=time.gmtime(); print \"%d-%d\" % (t.tm_year, t.tm_mon//4+1)')@silbe.org>\""
>        tg-send-to-ml-single = "!tg mail -s \"-p --stat --add-header=\\\"Reply-To: Sascha Silbe <sascha-ml-reply-to-$(python -c 'import time; t=time.gmtime(); print \"%d-%d\" % (t.tm_year, t.tm_mon//4+1)')@silbe.org>\\\"\""
> [sendemail]
>        from = Sascha Silbe <sascha-pgp at silbe.org>
>        chainreplyto = false
>        signedoffcc = false
>        suppressfrom = true
>        confirm = always
> [color]
>        diff = auto
>
> And this is (part of) my .git/config for sugar:
>
> [format]
>        headers = Mail-Followup-To: <sugar-devel at lists.sugarlabs.org>
> [sendemail]
>        to = sugar-devel <sugar-devel at lists.sugarlabs.org>
>        envelopesender = sascha-ml-ui-sugar-devel at silbe.org
>
>
> Sascha
>
> --
> http://sascha.silbe.org/
> http://www.infra-silbe.de/
>
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel
>
>