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

Delete this message

Reply to this message
Author: Alan
Date:  
To: tails-dev
Subject: Re: [Tails-dev] Please review and merge again feature/6342-update-camouflage-for-gnome3
Hi,

> 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.


Thanks

> I found some issues,
> though, for which I've opened the following tickets:
>
> Blockers (children of #6342):
>

Next time please put the title of the bugs in the email, it would be
easier to discuss

> https://labs.riseup.net/code/issues/7398


(Fix battery systray icons in Windows 8 camouflage)

Should be fixed but not yet tested.

> https://labs.riseup.net/code/issues/7401


(Fix web browser's close tab buttons in Windows 8 camouflage)

I investigated the issue but didn't fix it yet. See the ticket

> https://labs.riseup.net/code/issues/7402


(Fix task list icon for Nautilus in Windows 8 camouflage)

> Currently it's GNOME's default.


No. Currently it's the folder's icon coming from the theme. That's set
by nautilus. I don't think this is easy to change without changing
folder's icon nor it is a big issue.

I also fixed this one:
Bug #7382: Customized icons are way too small in the applications menu
https://labs.riseup.net/code/issues/7382

> Non-blockers (orphans):
>

I see they are now children of #7377, good!

> 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?
>

I'm giving a try, see Bug #7407: Set at least icon theme before
gnome-settings-daemon start (https://labs.riseup.net/code/issues/7407)

> > commit 00e20cd42d059159a1b56534ce670cc4f8b74d7e
> > [...]
> > +ls -s "/usr/share/pixmaps/gpgApplet/48x48-grey"
> "/usr/share/pixmaps/gpgApplet/48x48"
>
> Looks like you meant `ln -s ...`.
>

I think it is fixed by another commit, as the current file is good.

> >> #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.
>

OK