Re: [gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[David Seifert]

On Sun, 2019-10-20 at 19:45 +0200, Ulrich Mueller wrote:
> > > > > > On Sun, 20 Oct 2019, David Seifert wrote:
> > -		if [[ -e fonts.alias ]]; then
> > -			doins fonts.alias || die "failed to install
> > fonts.alias" # TODO old EAPI cleanup
> > -		fi
> > +		[[ -e fonts.alias ]] && doins fonts.alias
> >  	fi
> >  }
>
> Is the function's return value of any importance? The function will
> now
> return shell false if fonts.alias doesn't exist, while previously it
> returned true.

No.

>
> > -	[[ -n ${DOCS} ]] && { dodoc ${DOCS} || die "docs installation
> > failed" ; } # TODO old EAPI cleanup
> > +	einstalldocs
>
> >  	# install common docs
> > -	for commondoc in COPYRIGHT README{,.md,.txt} NEWS AUTHORS BUGS
> > ChangeLog FONTLOG.txt; do
> > +	for commondoc in COPYRIGHT FONTLOG.txt; do
> >  		[[ -s ${commondoc} ]] && dodoc ${commondoc}
> >  	done
>
> This changes the set of installed files, if the DOCS variable is
> defined. Is that intentional?
>

You mean if it's *not* defined? Yes, that's intentional in line with
making eclasses behave around DOCS consistently. If that leads to
installing READMEwin32.txt, then so be it. Importantly, this allows
using DOCS as an array without copypasta-ing around the einstalldocs
whitespace-list-vs-array handling code.

> > -			if [[ -e
> > "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]]; then
> > -				elog "  ${conffile##*/}"
> > -			fi
> > +			[[ -e
> > "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]] && elog
> > "  ${conffile##*/}"
>
> This doesn't change any functionality, but it adds an overlong line
> for
> no good reason.

The idea was to avoid if statements if you can use the more succinct
form.

>
> Ulrich