From: Duncan <1i5t5.duncan@cox.net>
To: gentoo-dev@lists.gentoo.org
Subject: [gentoo-dev] Re: gstreamer eclass review
Date: Sun, 18 Nov 2012 20:45:29 +0000 (UTC) [thread overview]
Message-ID: <pan.2012.11.18.20.45.31@cox.net> (raw)
In-Reply-To: 1353265590.23950.13.camel@kanae
Gilles Dartiguelongue posted on Sun, 18 Nov 2012 20:06:30 +0100 as
excerpted:
It's admittedly a style thing thus pretty much up to the author, purely
bikeshedding in the original sense of the essay's trivial color choice,
but...
> # Even though xz-utils are in @system, they must still be added to DEPEND; see
> # http://archives.gentoo.org/gentoo-dev/msg_a0d4833eb314d1be5d5802a3b710e0a4.xml
> if [[ ${GST_TARBALL_SUFFIX} == "xz" ]]; then
> DEPEND="${DEPEND} app-arch/xz-utils"
> fi
Single-clause conditional tests (without an else clause) such as the above
can be trivially rewritten like so:
[[ ${GST_TARBALL_SUFFIX} == "xz" ]] && \
DEPEND="${DEPEND} app-arch/xz-utils"
Two lines (or one if short enough) instead of three and more concise (fi
line omitted entirely, "if" omitted, "; then" replaced with &&).
It's possible to do similar with if/then/else -> &&/||, but that may
require {} for clarity if not bash parsing, and IMO is not nearly as
clear as the more straightforward no-else-clause case. So if there's
an else clause, I prefer if/then/else; if not, I prefer the && or || logic.
> if [[ ${PN} != ${GST_ORG_MODULE} ]]; then
> # Do not run test phase for invididual plugin ebuilds.
> RESTRICT="test"
> fi
Another example, this one definitely a one-liner (plus comment,
"invididual" typo left intact)...
# Do not run test phase for invididual plugin ebuilds.
[[ ${PN} != ${GST_ORG_MODULE} ]] && RESTRICT="test"
Or, avoiding that ! in the test and changing the && to ||...
[[ ${PN} == ${GST_ORG_MODULE} ]] || RESTRICT="test"
Also, while we're here, "test" is a string-literal with no possibility of
spaces or anything else that could otherwise confuse bash, so needs no
quotes. And I'm not sure if the != pattern matching trigger was
deliberate or not (see bash's "help [[" output). Thus...
[[ ${PN} = ${GST_ORG_MODULE} ]] || RESTRICT=test
... or...
[[ ${PN} == ${GST_ORG_MODULE} ]] || RESTRICT=test
... with the single vs. double equals making the string match (single) vs
pattern match (double) explicit. If the test-internal-not form is
retained, the string match form would be...
[[ ${PN} ! = ${GST_ORG_MODULE} ]] && RESTRICT=test
... while the pattern match form would retain the != that was used
(the distinction being the separating space, != is a pattern match
as is ==, ! = is a string match as is a single = ).
> # added to remove circular deps # 6/2/2006 - zaheerm
if [[ ${PN} != ${GST_ORG_MODULE} ]]; then
> RDEPEND="${RDEPEND} media-libs/${GST_ORG_MODULE}:${SLOT}"
> fi
Ditto... Further examples skipped.
> # @FUNCTION: gst-plugins10_src_install gst-plugins10_src_install() {
> gst-plugins10_find_plugin_dir
>
> if has "${EAPI:-0}" 0 1 2 3 ; then
> emake install DESTDIR="${D}" || die
> [[ -e README ]] && dodoc README
> else
> default
> fi
>
> [[ ${GST_LA_PUNT} = "yes" ]] && prune_libtool_files --modules
> }
Here (last line before the function-closing "}", as I said above, I prefer
if/then/else where there's an else clause, so wouldn't change the upper
conditional) we see a counter-example, doing it just as I suggested.
The if/then version (keeping function indent) would look like this.
if [[ ${GST_LA_PUNT} = "yes" ]]; then
prune_libtool_files --modules
fi
Of course regardless of that, the quotes around "yes" may be omitted as
it's a string literal. (And here, the explicit single-equals string-
literal match is used, as opposed to the double-equals pattern match. Of
course either one would work here as it's a trivial pattern, just noting
it for consistency.)
[[ ${GST_LA_PUNT} = yes ]] && prune_libtool_files --modules
But as I said up top, that's (mostly, the pattern matching vs string
matching will occasionally bite if you're not on the lookout for it)
trivial style stuff. You may well prefer the way it is now. But in that
case, why the counter-example, omitting if/then? (You may of course
fairly point out that consistency is a trivial style thing too. =:^)
--
Duncan - List replies preferred. No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master." Richard Stallman
next prev parent reply other threads:[~2012-11-18 20:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-18 19:06 [gentoo-dev] gstreamer eclass review Gilles Dartiguelongue
2012-11-18 20:45 ` Duncan [this message]
2012-11-18 22:59 ` [gentoo-dev] " Gilles Dartiguelongue
2012-11-19 4:23 ` Duncan
2012-11-18 23:13 ` Alec Warner
2012-11-21 15:27 ` [gentoo-dev] " Tomáš Chvátal
2012-11-21 20:54 ` Gilles Dartiguelongue
2012-12-06 4:01 ` Maxim Kammerer
2012-12-06 8:40 ` Gilles Dartiguelongue
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pan.2012.11.18.20.45.31@cox.net \
--to=1i5t5.duncan@cox.net \
--cc=gentoo-dev@lists.gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox