Code review comment for lp:~wookey/xdeb/xdeb-archfix

On Thu, Oct 07, 2010, Wookey wrote:
> Can I wrap on a dot or only on spaces? I thought about it but thought
> bin_cache = get_output(['apt-cache', apt_opts, 'show',
> binary]).splitlines()
>
> was harder to read
>
> and assumed that
> bin_cache = get_output(['apt-cache', apt_opts, 'show', binary])
> .splitlines()

    bin_cache = get_output(['apt-cache',
                            '-oAPT::Architecture=%s' % options.architecture,
                            'show',
                            binary]).splitlines()

 Or:
    apt_cache = ['apt-cache', 'show', binary]
    apt_cache.append('-oAPT::Architecture=%s' % options.architecture)
    bin_cache = get_output(apt_cache).splitlines()

 (untested)

> Can python use 2-char indenting instead of 4? That would save a load
> of space.

 If you're playing these tricks, you have indented too much and need to
 refactor for readibility; moving to 2-char indents deteriorates
 readibility

> Why is it better to wait until a duff download is attempted rather
> than check for invalid combination of options after reading the command
> line and config options? Checking up front saves waiting until the
> fault actually occurs, so the user gets immediate feedback. I suppose
> it's possible that no native downloads would be attempted so the issue
> wouldn't trigger - is that your thinking?

 Because maintaining the piece of logic which knows when native imports
 are going to happen is not going to be easy over time; the closer the
 test is to the use of the variable, the better

 I think we should just arrange for this situation not to happen by
 providing the right native_import URLs in the default config or have
 some good fallback mechanism

> To run pdebuild-cross on a package from the alip package list I have to do this:
> sourcepackage=apt-cache show $package | awk '/Source: .*/ {print $2}'
> if [ -n sourcepackage ]; then sourcepackage=$package; fi

 this means that if you pass linux to the above, you'll actually
 cross-build linux-meta. If you take both package types (source and
 binary), you can break expectations both ways, but if you only expect
 source packages you can't break expectations; also, it means we can
 save us this subtlety in xdeb which has experienced users to start with
 (as you just demonstrated)

--
Loïc Minier