FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 12-04-2007, 08:24 PM
Jim Ramsay
 
Default gentoo-x86 commit in eclass: rox-0install.eclass

Donnie Berkholz wrote:
> I don't remember this going by gentoo-dev. Always send eclasses to
> gentoo-dev before committing them.

My apologies, I was just so excited that I'd finally finished it!

Apparently I missed a few things, thanks very much for catching
it.

> > 0install_native_feed() {
> > local src="${1}"; shift
> > local path="${1}"; shift
>
> This is a rather strange paradigm to me, instead of:
>
> local src=$1 path=$2
> shift 2

This is cleaner, thanks. In fact, I don't even really need the
shift at all.

> > local feedfile=$(basename "${src}")
>
> You could do this in pure bash, although it doesn't really matter:
>
> local feedfile=${src##*/}

Sure, may as well, save a subshell.

> > local dest="${path}/$feedfile"
>
> How do you decide when not to use braces { } around variables?

In general I've used them everywhere... except there Fixed.

> > 0distutils "${src}" > tmp.native_feed || die "0distutils feed edit failed"
> >
> > if [[ ${ZEROINSTALL_STRIP_REQUIRES} ]]; then
> > # Strip out all 'requires' sections
> > sed -i -e '/<requires.*/>/d'
> > -e '/<requires.*>/,/</requires>/d' tmp.native_feed
>
> What happens if the contents of a <requires> section are on a separate
> line? Is this a concern?

It hasn't been so far - The convention in all known cases is
either it's all on one line, or a multi-line as is caught by the
second sed expression. This is just a stopgap measure until I
rework 0distutils to do this optional stripping on its own.

> > local feedname
>
> You could just declare feedname local and set it in the same line.

Not if I want to potentially die on the assignment. As I found
out in #gentoo-dev-help today, try this:

$ t() { local a=$(false) || echo "Die"; }; t

Versus this:

$ t() { local a; a=$(false) || echo "Die"; }; t
Die

> > feedname=$(0distutils -e "${src}") || "0distutils URI escape failed"
>
> What's the || doing? You've got a string sitting there. Is 'die'
> missing?

Verily - nice catch!

> > if ! $installed; then
>
> This is kind of a weird way to do it. I'd check instead for
> [[ -n ${installed} ]] and initialize it to empty.

Sure, that looks nicer.

I'll be committing these changes right away, since the "die" ones
at least are very important. But I'll be submitting further
changes to the list first. Sorry about that

Luckily only 1 ebuild so far which uses this eclass has actually
hit the tree!

--
Jim Ramsay
Gentoo/Linux Developer (rox/fluxbox/gkrellm)
 

Thread Tools




All times are GMT. The time now is 12:20 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org