From: Alec Warner <antarus@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Subject: Re: [gentoo-dev] Re: gstreamer eclass review
Date: Sun, 18 Nov 2012 15:13:34 -0800 [thread overview]
Message-ID: <CAAr7Pr97C9y=+-zaH_5FRErBYMqsG9RgD_hnc42CMi7X8m_vUQ@mail.gmail.com> (raw)
In-Reply-To: <pan.2012.11.18.20.45.31@cox.net>
On Sun, Nov 18, 2012 at 12:45 PM, Duncan <1i5t5.duncan@cox.net> wrote:
> 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 &&).
http://www.quickmeme.com/meme/3ru1zx/
>
> 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 23:16 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 ` [gentoo-dev] " Duncan
2012-11-18 22:59 ` Gilles Dartiguelongue
2012-11-19 4:23 ` Duncan
2012-11-18 23:13 ` Alec Warner [this message]
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='CAAr7Pr97C9y=+-zaH_5FRErBYMqsG9RgD_hnc42CMi7X8m_vUQ@mail.gmail.com' \
--to=antarus@gentoo.org \
--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