From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 24F861381F3 for ; Sun, 18 Nov 2012 20:46:41 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 9C80121C001; Sun, 18 Nov 2012 20:46:30 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 78F2DE0653 for ; Sun, 18 Nov 2012 20:45:52 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with ESMTP id A8F7E33BF3F for ; Sun, 18 Nov 2012 20:45:51 +0000 (UTC) X-Virus-Scanned: by amavisd-new using ClamAV at gentoo.org X-Spam-Flag: NO X-Spam-Score: -1.465 X-Spam-Level: X-Spam-Status: No, score=-1.465 tagged_above=-999 required=5.5 tests=[AWL=-1.348, RP_MATCHES_RCVD=-0.115, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001] autolearn=no Received: from smtp.gentoo.org ([IPv6:::ffff:127.0.0.1]) by localhost (smtp.gentoo.org [IPv6:::ffff:127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aM3QDFjLpoYv for ; Sun, 18 Nov 2012 20:45:45 +0000 (UTC) Received: from plane.gmane.org (plane.gmane.org [80.91.229.3]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id 228DA33BF3C for ; Sun, 18 Nov 2012 20:45:43 +0000 (UTC) Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1TaBkU-0005lk-Nq for gentoo-dev@gentoo.org; Sun, 18 Nov 2012 21:45:50 +0100 Received: from ip68-231-22-224.ph.ph.cox.net ([68.231.22.224]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sun, 18 Nov 2012 21:45:50 +0100 Received: from 1i5t5.duncan by ip68-231-22-224.ph.ph.cox.net with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sun, 18 Nov 2012 21:45:50 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: gentoo-dev@lists.gentoo.org From: Duncan <1i5t5.duncan@cox.net> Subject: [gentoo-dev] Re: gstreamer eclass review Date: Sun, 18 Nov 2012 20:45:29 +0000 (UTC) Message-ID: References: <1353265590.23950.13.camel@kanae> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: ip68-231-22-224.ph.ph.cox.net User-Agent: Pan/0.140 (Chocolate Salty Balls; GIT f91bd24 /usr/src/portage/src/egit-src/pan2) X-Archives-Salt: bc699b76-e484-4597-932f-bbcfae9f883c X-Archives-Hash: cd0ebab29ea08830573f23fe49c34fc0 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