Re: [Tails-dev] Please review and merge again feature/6342-u…

Delete this message

Reply to this message
Author: anonym
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] Please review and merge again feature/6342-update-camouflage-for-gnome3
10/06/14 13:47, Alan wrote:
> Hi,
>
> I belive the blockers are now fixed.
>
> Please review and merge again feature/6342-update-camouflage-for-gnome3
> into devel, or if you prefer review and merge the following specific
> branches (each fixing the bug whose number is in its name):


I'll go with feature/6342-update-camouflage-for-gnome3 instead of the
split branches. Oh, and before I forget, great job! Now I'm starting to
get really convinced, when I take a few steps back from my display,
close my eyes for a bit, and have quick look.

As for a visual review, it looks much better now. I found some issues,
though, for which I've opened the following tickets:

Blockers (children of #6342):

https://labs.riseup.net/code/issues/7398
https://labs.riseup.net/code/issues/7401
https://labs.riseup.net/code/issues/7402
https://labs.riseup.net/code/issues/7403

Non-blockers (orphans):

https://labs.riseup.net/code/issues/7397
https://labs.riseup.net/code/issues/7399
https://labs.riseup.net/code/issues/7400

As for the code review of the
origin/devel..origin/feature/6342-update-camouflage-for-gnome3 delta,
here goes:

> commit c5996cc08152e3106b402446ec021649dcad5273
> [...]
> +# XXX Ugly restart hacks because this script runs too late
> [...]


I'm wondering if it's wise to use xdg for running these script if it's
generally too late. In particular, I'm afraid of yet undiscovered race
conditions vs other GNOME components we haven't discovered yet. Why not
call the scripts from Tails Greeter's PostLogin (including some `sudo -u
$LIVE_USER` stuff for the activation script), like before?

> commit 00e20cd42d059159a1b56534ce670cc4f8b74d7e
> [...]
> +ls -s "/usr/share/pixmaps/gpgApplet/48x48-grey"

"/usr/share/pixmaps/gpgApplet/48x48"

Looks like you meant `ln -s ...`.

>> #7274: Decide if torbrowser camouflage is still needed
>>
> Done (comments welcome, improvements possible...)


That's what I'm talking about, low-hanging fruit. :) It looks good
enough to me.

Cheers!