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

Delete this message

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

On Thu, 13 Jun 2013 13:55:40 +0200 intrigeri <intrigeri@???> wrote:
> Alan wrote (12 Jun 2013 19:41:23 GMT) :
> >> Yes I will do it for 0.19.
> >>
> > It was laying on my storage device... it's now mergeable. Likely to
> > be too late for 0.19, sorry.
>
> >    args = ["apt-get", "--quiet", "--yes"]

>
> This looks surprising a nomenclature to me. I'm more used to apt-get
> being called the program, and --quiet --yes being called its
> arguments. I don't think it's a big deal or a blocker, but I do think
> it ought to be fixed for better robustness against future regressions.
>

I used the same name as in python doc
(http://docs.python.org/2/library/subprocess.html):

    args should be a sequence of program arguments or else a single
    string. By default, the program to execute is the first item in
    args if args is a sequence. If args is a string, the interpretation
    is platform-dependent and described below. See the shell and
    executable arguments for additional differences from the default
    behavior. Unless otherwise stated, it is recommended to pass args
    as a sequence.


Do you still think I should change this name?g

> I'm declaring this branch passes a static review, congrats!
>

Thanks

> However, during my basic testing, I could not get this feature work at
> all on a Tails built from experimental, due to non-existing
> /lib/live/mount/persistence/*_unlocked/ (looks like
> /live/persistence/*_unlocked/ might work better), so I had a hard time
> believing it was tested at all in anything that looks like real
> settings, so I felt like I was wasting my time on untested stuff at
> the worst possible time, so I dropped the reviewing before I felt more
> pissed off than I'd like to feel.
>

It was tested on 0.18 + feature/remember_installed_packages. It's right
that I didn't test it in expermental after merging it there. It was
however unclear to me that I was supposed to do it. We might want to
make clear against which branch one should test a feature before asking
for a merge.

> Does anyone want to provide a minimal and tested fix for that
> regression in time for 0.19 final, or should we add this feature
> brokenness to known issues? Or perhaps this feature is not broken in
> the devel branch, but only in experimental? Or perhaps I did my
> testing the wrong way, and it's actually not broken?
>

I'll take care of that before the 22nd.

Cheers