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

Delete this message

Reply to this message
Author: Alan
Date:  
To: tails-dev
Subject: Re: [Tails-dev] Please review and merge feature/liveusb_ui_improvement
Hi,

On Wed, 25 Sep 2013 19:41:19 +0200 intrigeri <intrigeri@???> wrote:
> Alan wrote (25 Sep 2013 15:12:34 GMT) :
> > I believe the feature/liveusb_ui_improvement is now ready to merge.
>
> Great work! Here we go.
>

Thanks.

I'm only answering below to things you didn't already fixed.

> Minor process note: next time, please reassign to the current RM when
> asking for a merge. Thanks in advance.
>

I didn't know sorry. Seems like we should update our doc (see other
email).

> As a bonus, it seems that you've implemented #6258 (Mention that the
> persistent volume will be preserved while upgrading). Great!
>
> So I've changed it to:
>
> You are going to upgrade Tails on the %s %s device.
>
> ... and even to:
>
> You are going to upgrade Tails on the %(vendor)s %(model)s device.
>
> ... to ease translators' work, in particular for RTL languages.
>

Nice, I didn't know about this syntax.

> Just curious: any specific reason that I've missed to display the
> device size in the confirmation dialog on initial install, but not at
> upgrade time?
>

No.

> > -                self.status(_("Warning: All data on the selected
> > drive will "
> > -                              "be lost."))

>
> Ouch. I strongly disagree with dropping this information, when we're
> going to partition the destination storage device, so I've
> implemented:
>
>    _("You are going to install Tails on the %(vendor)s %(model)s
> device (%(size)s). " "All data on the selected drive will be lost. "
>      "Continue?")

>

I'm afraid that the phrasing make the suer believe that her data is
actually erased, which is not the case, but that's not a regression.

> > +def _format_bytes(value):
> > +    return '%0.1f GB' % (value / 10.0**9)

>
> I appreciate the minor refactoring, but then I'd rather:
>
>   * either use some library to display human-readable units in all
>     cases (think: 2TB external drive) to improve UX
>   * or simply rename the function to _format_bytes_in_GB, so that at
>     least the name expresses its current limitations.

>

I would choose the 2nd for now.

> However, the No/Yes buttons ordering seems unusual to me. I didn't
> dare patching it, as I thought you might have followed the GNOME HIG
> or something, and I may be missing something. Any reason to order it
> this way?
>

No. I'm used to GTK+, which chooses the right order automatically. Qt
might not do that.

> The device choice menu doesn't fit, and has an ugly "go down" button
> when I:
> 1. start the GUI with only one possible device plugged in
> 2. plug a second device
> 3. open the menu
> It's not as painful to use as the greeter language menu, but still.
> I can't reproduce this is the target device is plugged before I start
> the installer. Can you reproduce this?


I'll have a look.

> To end with, the test suite was not updated yet, and is likely broken
> by these changes, so please file a ticket about it, assigned to me if
> you wish (I'd rather do it together, though). This must be fixed
> before the 0.21 freeze.
>

I'm afraid that I don't have the infrastructure to do that yet, so I
would appreciate if you took care of it.

Cheers