[gentoo-dev] RFC: leechcraft.eclass

[gentoo-dev] RFC: leechcraft.eclass

[Maxim Koltsov]

Hi devs,
I'm about to add Leechcraft modular internet client to tree. It has 32
packages and uses it's own eclass. Please review it and allow me to
commit it to the tree.
Also i'd want to ask: is it woth to add new category (e.g.
leechcraft-plugins) to simplify managing leechcraft ebuilds. And the
last question: is it good to add 9999 versions for all ebuilds too?

[gentoo-dev] Re: RFC: leechcraft.eclass

[Maxim Koltsov]

[-- Attachment #1: Type: text/plain, Size: 90 bytes --]

Sorry, forgot the eclass. Attaching it here...
P.S. Email of author: 0xd34df00d@gmail.com

[-- Attachment #2: leechcraft.eclass --]
[-- Type: application/octet-stream, Size: 1865 bytes --]

# Copyright 1999-2011 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: 22.02.2010 slepnoga v1.002 $


# Original author: 0xd34df00d <0xd34df00d@gmail.com> and
#				   Andrian Nord <NightNord@niifaq.ru>

# Commiter: A.Vinogradov aka slepnoga <slep@vinogradov.lv>


# Only EAPI >1 supported

case ${EAPI:-0} in
        4|3|2) ;;
		0|1) die "EAPI not supported, bug ebuild mantainer" ;;
		*) die "Unknown EAPI, Bug eclass maintainers." ;;
esac

# Exported functions
# Logic: for live ebuild call standart and src_unpack;
LEECHCRAFT_EXPR="src_configure src_install"

inherit cmake-utils

if [[ "${PV}" == "9999" ]]; then
	EGIT_REPO_URI="git://github.com/0xd34df00d/leechcraft.git"
	EGIT_PROJECT="leechcraft-${PV}"
	LEECHCRAFT_EXPR="${LEECHCRAFT_EXPR} src_unpack"	
	KEYWORDS=""

	inherit git-2
else
	SRC_URI="mirror://sourceforge/leechcraft/leechcraft-${PV}.tar.bz2"
	MY_P='leechcraft'
	S="${WORKDIR}/${MY_P}-${PV}"
	KEYWORDS="~amd64 ~x86"
fi

HOMEPAGE="http://leechcraft.org/"

DEPEND="${DEPEND}
		!www-client/leechcraft"

LICENSE="GPL-3"
SLOT="0"

CMAKE_MIN_VERSION="2.8"

EXPORT_FUNCTIONS ${LEECHCRAFT_EXPR}


# Set ${S} variable

if [[ ${PN} != "leechcraft-core" ]]; then
	CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
else
	CMAKE_USE_DIR="${S}/src"
fi

# @FUNCTION:leechcraft_src_unpack
# @DESCRIPTION:
# Standart src_unpack live ebuild

leechcraft_src_unpack() {
	git-2_src_unpack

	cd "${S}"
}

# @FUNCTION: leechcraft_src_configure
# @DESCRIPTION:
# Use for configure leechcraft source.
# Build_type is magic :)

leechcraft_src_configure() {
	if use debug ; then
		CMAKE_BUILD_TYPE="RelWithDebInfo"
	else
		CMAKE_BUILD_TYPE="Release"
	fi

	cmake-utils_src_configure
}

# @FUNCTION: leechcraft_src_install
# @DESCRIPTION:
# Call cmake-utils_src_install :)

leechcraft_src_install() {
	cmake-utils_src_install
}

Re: [gentoo-dev] RFC: leechcraft.eclass

[Marc Schiffbauer]

Am Freitag, 22. Juli 2011, 14:50:06 schrieb Maxim Koltsov:
> Hi devs,
> I'm about to add Leechcraft modular internet client to tree. It has 32
> packages and uses it's own eclass. Please review it and allow me to
> commit it to the tree.
> Also i'd want to ask: is it woth to add new category (e.g.
> leechcraft-plugins) to simplify managing leechcraft ebuilds. And the
> last question: is it good to add 9999 versions for all ebuilds too?

IMO live ebuilds should only be held in an overlay.

-Marc

Re: [gentoo-dev] RFC: leechcraft.eclass

[Alex Alexander]

On Fri, Jul 22, 2011 at 14:21, Marc Schiffbauer <mschiff@gentoo.org> wrote:
> Am Freitag, 22. Juli 2011, 14:50:06 schrieb Maxim Koltsov:
>> Hi devs,
>> I'm about to add Leechcraft modular internet client to tree. It has 32
>> packages and uses it's own eclass. Please review it and allow me to
>> commit it to the tree.
>> Also i'd want to ask: is it woth to add new category (e.g.
>> leechcraft-plugins) to simplify managing leechcraft ebuilds. And the
>> last question: is it good to add 9999 versions for all ebuilds too?
>
> IMO live ebuilds should only be held in an overlay.
>
> -Marc

9999 versions are nice, but they typically require more time and
effort to maintain. I'd recommend adding them only if you are willing
to do the work. Sometimes 9999 ebuilds are useful as a way to prepare
for the next release.

-- 
Alex Alexander | wired
+ Gentoo Linux Developer
++ www.linuxized.com

Re: [gentoo-dev] RFC: leechcraft.eclass

[Chí-Thanh Christopher Nguyễn]

Maxim Koltsov schrieb:
> Also i'd want to ask: is it woth to add new category (e.g.
> leechcraft-plugins) to simplify managing leechcraft ebuilds. And the
> last question: is it good to add 9999 versions for all ebuilds too?
>   

I don't think there is a need for a new category. But I do think it is
good to have live ebuilds in portage, especially for fast moving
applications. If you are very close to upstream (as it seems to be) then
you can submit patches directly there which makes maintaining the live
ebuilds easier.


Best regards,
Chí-Thanh Christopher Nguyen

Re: [gentoo-dev] RFC: leechcraft.eclass

[Rich Freeman]

2011/7/22 Chí-Thanh Christopher Nguyễn <chithanh@gentoo.org>:
> But I do think it is
> good to have live ebuilds in portage, especially for fast moving
> applications.

I think they can be useful for any application as long as they're
maintained.  I can see the value of the QA policy that they must be
masked, but I don't see value in forcing them out of the tree.  I
think the key is that we want the tree to work - if it works then it
belongs.  The ability to seamlessly support live builds is one of
Gentoo's distinctive features - since we're already a bit of a niche
we should embrace this.

Rich

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

On Fri, 22 Jul 2011 14:57:08 +0400
Maxim Koltsov <maksbotan@gentoo.org> wrote:

> Sorry, forgot the eclass. Attaching it here...
> P.S. Email of author: 0xd34df00d@gmail.com

Then you should CC him (like I did now).

> # Original author: 0xd34df00d <0xd34df00d@gmail.com> and
> #				   Andrian Nord <NightNord@niifaq.ru>
> 
> # Commiter: A.Vinogradov aka slepnoga <slep@vinogradov.lv>

Use eclassdoc.

> case ${EAPI:-0} in
>         4|3|2) ;;
> 		0|1) die "EAPI not supported, bug ebuild mantainer" ;;
> 		*) die "Unknown EAPI, Bug eclass maintainers." ;;
> esac

Don't mix space and tab indent.

> if [[ "${PV}" == "9999" ]]; then

Needs not to be quoted.

> 	EGIT_PROJECT="leechcraft-${PV}"

What's the reasoning for this?

> 	KEYWORDS=""

Keywords must not be set by an eclass. Package managers will complain
about this.

> 	MY_P='leechcraft'

Usually, ${MY_P} stands for [alternate PN]-[PV]. MY_PN's what you're
referring to here.

> 	S="${WORKDIR}/${MY_P}-${PV}"

You seem not to be using ${MY_P} anywhere else. Just put
'leechcraft-${PV}' there then, don't pollute the environment.

> 	KEYWORDS="~amd64 ~x86"

As above, completely illegal.

> DEPEND="${DEPEND}
> 		!www-client/leechcraft"

What's this and why is that?

> SLOT="0"

SLOTs don't fit eclasses too, unless special use. Eclass is no excuse
to avoid declaring standard variables in ebuilds.

> # @FUNCTION:leechcraft_src_unpack
> # @DESCRIPTION:
> # Standart src_unpack live ebuild
> 
> leechcraft_src_unpack() {
> 	git-2_src_unpack
> 
> 	cd "${S}"
> }

What's the point of redeclaring this? Inheriting git-2 should do
the same, wouldn't it?

> # @FUNCTION: leechcraft_src_configure
> # @DESCRIPTION:
> # Use for configure leechcraft source.
> # Build_type is magic :)

That's not an useful description.

> # @FUNCTION: leechcraft_src_install
> # @DESCRIPTION:
> # Call cmake-utils_src_install :)
> 
> leechcraft_src_install() {
> 	cmake-utils_src_install
> }

Once again, inheriting cmake-utils should do that.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] RFC: leechcraft.eclass

[Marc Schiffbauer]

[-- Attachment #1: Type: text/plain, Size: 1376 bytes --]

* Alex Alexander schrieb am 22.07.11 um 13:30 Uhr:
> On Fri, Jul 22, 2011 at 14:21, Marc Schiffbauer <mschiff@gentoo.org> wrote:
> > Am Freitag, 22. Juli 2011, 14:50:06 schrieb Maxim Koltsov:
> >> Hi devs,
> >> I'm about to add Leechcraft modular internet client to tree. It has 32
> >> packages and uses it's own eclass. Please review it and allow me to
> >> commit it to the tree.
> >> Also i'd want to ask: is it woth to add new category (e.g.
> >> leechcraft-plugins) to simplify managing leechcraft ebuilds. And the
> >> last question: is it good to add 9999 versions for all ebuilds too?
> >
> > IMO live ebuilds should only be held in an overlay.
> >
> > -Marc
> 
> 9999 versions are nice, but they typically require more time and
> effort to maintain. I'd recommend adding them only if you are willing
> to do the work. Sometimes 9999 ebuilds are useful as a way to prepare
> for the next release.

Yes, but the big drawback is that you do not have any checksums of
the source. So if for example an upstream source code gets exploited you 
will never notice until the trojan or whatever got in there will do
something. Sure this can happen with normal tarballs too,
but is much more unlikely and can only happen if the source is
already bad at the time of "repoman manifest".

-Marc
-- 
8AAC 5F46 83B4 DB70 8317  3723 296C 6CCA 35A6 4134

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

On Thu, 18 Aug 2011 14:38:01 +0400
Maxim Koltsov <maksbotan@gentoo.org> wrote:

> We've fixed all the problems with eclass. Please review it and give OK
> for commit, i plan to do it this night.

> 	0|1) die "EAPI not supported, bug ebuild mantainer" ;;
> 	*) die "Unknown EAPI, Bug eclass maintainers." ;;

I think I already mentioned that. Keep consistent case, and in this
case lowercase 'bug' is correct'.

> # @ECLASS-VARIABLE: LC_PCAT
> # @DESCRIPTION:
> # Set this to the category of the plugin, if any.
> : ${LC_PCAT:=}

Please use verbose variable names, and prefix them with eclass
filename; e.g. LEECHCRAFT_PLUGIN_CATEGORY.

And I think @DEFAULT_UNSET may be of interest too.

> 
> if [ "${LC_PCAT+x}" != "x" ]; then
> 	CMAKE_USE_DIR="${S}/src/plugins/${LC_PCAT}/${PN#leechcraft-}"
> else
> 	if [[ ${PN} != "leechcraft-core" ]]; then
> 		CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
> 	else
> 		CMAKE_USE_DIR="${S}/src"
> 	fi
> fi

if-elif-else-fi.

And I think you dropped gentoo-dev@ from recipients a few mails ago.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Fabian Groffen]

On 18-08-2011 16:57:59 +0200, Michał Górny wrote:
> > # @ECLASS-VARIABLE: LC_PCAT
> > # @DESCRIPTION:
> > # Set this to the category of the plugin, if any.
> > : ${LC_PCAT:=}
> 
> Please use verbose variable names, and prefix them with eclass
> filename; e.g. LEECHCRAFT_PLUGIN_CATEGORY.

Really?  The python eclass is full of such awkward overly verbose names.
One can also exaggerate...

> > if [ "${LC_PCAT+x}" != "x" ]; then
> > 	CMAKE_USE_DIR="${S}/src/plugins/${LC_PCAT}/${PN#leechcraft-}"
> > else
> > 	if [[ ${PN} != "leechcraft-core" ]]; then
> > 		CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
> > 	else
> > 		CMAKE_USE_DIR="${S}/src"
> > 	fi
> > fi
> 
> if-elif-else-fi.

This sounds like a kind of bogus suggestion to me.  Mixing use of [ and
[[, on the other hand, is more what deserves attention here.



-- 
Fabian Groffen
Gentoo on a different level

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Alec Warner]

On Thu, Aug 18, 2011 at 8:27 AM, Fabian Groffen <grobian@gentoo.org> wrote:
> On 18-08-2011 16:57:59 +0200, Michał Górny wrote:
>> > # @ECLASS-VARIABLE: LC_PCAT
>> > # @DESCRIPTION:
>> > # Set this to the category of the plugin, if any.
>> > : ${LC_PCAT:=}
>>
>> Please use verbose variable names, and prefix them with eclass
>> filename; e.g. LEECHCRAFT_PLUGIN_CATEGORY.
>
> Really?  The python eclass is full of such awkward overly verbose names.
> One can also exaggerate...

python.eclass, perhaps not the best style to use as a defense ;)

-A

>
>> > if [ "${LC_PCAT+x}" != "x" ]; then
>> >     CMAKE_USE_DIR="${S}/src/plugins/${LC_PCAT}/${PN#leechcraft-}"
>> > else
>> >     if [[ ${PN} != "leechcraft-core" ]]; then
>> >             CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
>> >     else
>> >             CMAKE_USE_DIR="${S}/src"
>> >     fi
>> > fi
>>
>> if-elif-else-fi.
>
> This sounds like a kind of bogus suggestion to me.  Mixing use of [ and
> [[, on the other hand, is more what deserves attention here.
>
>
>
> --
> Fabian Groffen
> Gentoo on a different level
>
>

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Maxim Koltsov]

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

2011/8/18 Michał Górny <mgorny@gentoo.org>:
> On Thu, 18 Aug 2011 14:38:01 +0400
> Maxim Koltsov <maksbotan@gentoo.org> wrote:
>
>
>>       0|1) die "EAPI not supported, bug ebuild mantainer" ;;
>>       *) die "Unknown EAPI, Bug eclass maintainers." ;;
>
> I think I already mentioned that. Keep consistent case, and in this
> case lowercase 'bug' is correct'.

Sorry, i lost fixed version and forgot to correct this in new one :(

>> # @ECLASS-VARIABLE: LC_PCAT
>> # @DESCRIPTION:
>> # Set this to the category of the plugin, if any.
>> : ${LC_PCAT:=}
>
> Please use verbose variable names, and prefix them with eclass
> filename; e.g. LEECHCRAFT_PLUGIN_CATEGORY.
>
> And I think @DEFAULT_UNSET may be of interest too.
done

> if-elif-else-fi.
done

> And I think you dropped gentoo-dev@ from recipients a few mails ago.
Yes, added it now.

-- 
Regards, Maxim.

[-- Attachment #2: leechcraft.eclass --]
[-- Type: application/octet-stream, Size: 1663 bytes --]

# Copyright 1999-2011 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: 22.02.2010 slepnoga v1.002 $
#
# @ECLASS: leechcraft.eclass
# @MAINTAINER:
# maksbotan@gentoo.org
# 0xd34df00d@gmail.com
# @BLURB: Common functions and setup utilities for the LeechCraft app
# @DESCRIPTION:
# The leechcraft eclass contains a common set of functions and steps
# needed to build LeechCraft core or its plugins.
# Though this eclass seems to be small at the moment, it seems like a
# good idea to make all plugins inherit from it, since all plugins
# have mostly the same configuring/build process.
# Thanks for original eclass to Andrian Nord <NightNord@niifaq.ru>.
#
# Only EAPI >1 supported

case ${EAPI:-0} in
	4|3|2) ;;
	0|1) die "EAPI not supported, bug ebuild mantainer" ;;
	*) die "Unknown EAPI, bug eclass maintainers" ;;
esac

inherit cmake-utils flag-o-matic

if [[ ${PV} == 9999 ]]; then
	EGIT_REPO_URI="git://github.com/0xd34df00d/leechcraft.git"
	EGIT_PROJECT="leechcraft"

	inherit git-2
else
	SRC_URI="mirror://sourceforge/leechcraft/leechcraft-${PV}.tar.bz2"
	S="${WORKDIR}/leechcraft-${PV}"
fi

HOMEPAGE="http://leechcraft.org/"
LICENSE="GPL-3"

EXPORT_FUNCTIONS src_configure

# @ECLASS-VARIABLE: LEECHCRAFT_PACKAGE_CATEGORY
# @DEFAULT_UNSET
# @DESCRIPTION:
# Set this to the category of the plugin, if any.
: ${LEECHCRAFT_PACKAGE_CATEGORY:=}

if [ "${LEECHCRAFT_PACKAGE_CATEGORY+x}" != "x" ]; then
	CMAKE_USE_DIR="${S}/src/plugins/${LEECHCRAFT_PACKAGE_CATEGORY}/${PN#leechcraft-}"
elif [ ${PN} != "leechcraft-core" ]; then
		CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
else
		CMAKE_USE_DIR="${S}/src"
fi

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Maxim Koltsov]

2011/8/18 Fabian Groffen <grobian@gentoo.org>:
>> > if [ "${LC_PCAT+x}" != "x" ]; then
>> >     CMAKE_USE_DIR="${S}/src/plugins/${LC_PCAT}/${PN#leechcraft-}"
>> > else
>> >     if [[ ${PN} != "leechcraft-core" ]]; then
>> >             CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
>> >     else
>> >             CMAKE_USE_DIR="${S}/src"
>> >     fi
>> > fi
>>
>> if-elif-else-fi.
>
> This sounds like a kind of bogus suggestion to me.  Mixing use of [ and
> [[, on the other hand, is more what deserves attention here.

'elif' seems more convenient here, i just forgot about it :)

-- 
Regards, Maxim.

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

On Thu, 18 Aug 2011 19:30:29 +0400
Maxim Koltsov <maksbotan@gentoo.org> wrote:

> if [ "${LEECHCRAFT_PACKAGE_CATEGORY+x}" != "x" ]; then
> 	CMAKE_USE_DIR="${S}/src/plugins/${LEECHCRAFT_PACKAGE_CATEGORY}/${PN#leechcraft-}"
> elif [ ${PN} != "leechcraft-core" ]; then
> 		CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"

[[ ]] is better here because it's a bash builtin. And you don't need
quotes there.

What should happen if LEECHCRAFT_PACKAGE_CATEGORY is set to ''?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Maxim Koltsov]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

2011/8/18 Michał Górny <mgorny@gentoo.org>:
> On Thu, 18 Aug 2011 19:30:29 +0400
> Maxim Koltsov <maksbotan@gentoo.org> wrote:
>
>> if [ "${LEECHCRAFT_PACKAGE_CATEGORY+x}" != "x" ]; then
>>       CMAKE_USE_DIR="${S}/src/plugins/${LEECHCRAFT_PACKAGE_CATEGORY}/${PN#leechcraft-}"
>> elif [ ${PN} != "leechcraft-core" ]; then
>>               CMAKE_USE_DIR="${S}/src/plugins/${PN#leechcraft-}"
>
> [[ ]] is better here because it's a bash builtin. And you don't need
> quotes there.
>
> What should happen if LEECHCRAFT_PACKAGE_CATEGORY is set to ''?

Fixed, i think. In this version empty variable will be handled correctly.

-- 
Regards, Maxim.

[-- Attachment #2: leechcraft.eclass --]
[-- Type: application/octet-stream, Size: 1658 bytes --]

# Copyright 1999-2011 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: 22.02.2010 slepnoga v1.002 $
#
# @ECLASS: leechcraft.eclass
# @MAINTAINER:
# maksbotan@gentoo.org
# 0xd34df00d@gmail.com
# @BLURB: Common functions and setup utilities for the LeechCraft app
# @DESCRIPTION:
# The leechcraft eclass contains a common set of functions and steps
# needed to build LeechCraft core or its plugins.
# Though this eclass seems to be small at the moment, it seems like a
# good idea to make all plugins inherit from it, since all plugins
# have mostly the same configuring/build process.
# Thanks for original eclass to Andrian Nord <NightNord@niifaq.ru>.
#
# Only EAPI >1 supported

case ${EAPI:-0} in
	4|3|2) ;;
	0|1) die "EAPI not supported, bug ebuild mantainer" ;;
	*) die "Unknown EAPI, bug eclass maintainers" ;;
esac

inherit cmake-utils flag-o-matic

if [[ ${PV} == 9999 ]]; then
	EGIT_REPO_URI="git://github.com/0xd34df00d/leechcraft.git"
	EGIT_PROJECT="leechcraft"

	inherit git-2
else
	SRC_URI="mirror://sourceforge/leechcraft/leechcraft-${PV}.tar.bz2"
	S="${WORKDIR}/leechcraft-${PV}"
fi

HOMEPAGE="http://leechcraft.org/"
LICENSE="GPL-3"

EXPORT_FUNCTIONS src_configure

# @ECLASS-VARIABLE: LEECHCRAFT_PACKAGE_CATEGORY
# @DEFAULT_UNSET
# @DESCRIPTION:
# Set this to the category of the plugin, if any.
: ${LEECHCRAFT_PACKAGE_CATEGORY:=}

if [[ -z "${LEECHCRAFT_PACKAGE_CATEGORY}" ]]; then
	CMAKE_USE_DIR="${S}"/src/plugins/${LEECHCRAFT_PACKAGE_CATEGORY}/${PN#leechcraft-}
elif [[ ${PN} != "leechcraft-core" ]]; then
	CAKE_USE_DIR="${S}"/src/plugins/${PN#leechcraft-}
else
	CMAKE_USE_DIR="${S}"/src
fi

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

On Thu, 18 Aug 2011 22:33:15 +0400
Maxim Koltsov <maksbotan@gentoo.org> wrote:

> if [[ -z "${LEECHCRAFT_PACKAGE_CATEGORY}" ]]; then
> 	CMAKE_USE_DIR="${S}"/src/plugins/${LEECHCRAFT_PACKAGE_CATEGORY}/${PN#leechcraft-}

Dude, that's the opposite.

> elif [[ ${PN} != "leechcraft-core" ]]; then
> 	CAKE_USE_DIR="${S}"/src/plugins/${PN#leechcraft-}

Don't quote that. It looks bad that the left-side is unquoted and right
side is quoted.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Fabian Groffen]

On 18-08-2011 20:42:23 +0200, Michał Górny wrote:
> > elif [[ ${PN} != "leechcraft-core" ]]; then
> > 	CAKE_USE_DIR="${S}"/src/plugins/${PN#leechcraft-}
> 
> Don't quote that. It looks bad that the left-side is unquoted and right
> side is quoted.

it's a string, what's the problem?



-- 
Fabian Groffen
Gentoo on a different level

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On Thu, 18 Aug 2011 20:43:59 +0200
Fabian Groffen <grobian@gentoo.org> wrote:

> On 18-08-2011 20:42:23 +0200, Michał Górny wrote:
> > > elif [[ ${PN} != "leechcraft-core" ]]; then
> > > 	CAKE_USE_DIR="${S}"/src/plugins/${PN#leechcraft-}
> > 
> > Don't quote that. It looks bad that the left-side is unquoted and
> > right side is quoted.
> 
> it's a string, what's the problem?

It's just a matter of taste. I think it looks better if both sides are
quoted, or neither is.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Ulrich Mueller]

>>>>> On Thu, 18 Aug 2011, Michał Górny wrote:

> On Thu, 18 Aug 2011 20:43:59 +0200
> Fabian Groffen <grobian@gentoo.org> wrote:

>> > > elif [[ ${PN} != "leechcraft-core" ]]; then
>> > > 	CAKE_USE_DIR="${S}"/src/plugins/${PN#leechcraft-}
>> > 
>> > Don't quote that. It looks bad that the left-side is unquoted and
>> > right side is quoted.
>> 
>> it's a string, what's the problem?

> It's just a matter of taste. I think it looks better if both sides
> are quoted, or neither is.

But both sides of [[ ]] aren't symmetric, in the first place:

# When the == and != operators are used, the string to the right of
# the operator is considered a pattern and matched according to the
# rules described below under Pattern Matching.

So there's almost never a reason for quoting the left-hand side, while
on the right-hand side quotes will suppress pattern matching.

(Of course, in the example above it doesn't matter, because the string
doesn't contain any special characters.)

Ulrich

Re: [gentoo-dev] Re: RFC: leechcraft.eclass

[Fabian Groffen]

On 18-08-2011 21:41:47 +0200, Michał Górny wrote:
> On Thu, 18 Aug 2011 20:43:59 +0200
> Fabian Groffen <grobian@gentoo.org> wrote:
> 
> > On 18-08-2011 20:42:23 +0200, Michał Górny wrote:
> > > > elif [[ ${PN} != "leechcraft-core" ]]; then
> > > > 	CAKE_USE_DIR="${S}"/src/plugins/${PN#leechcraft-}
> > > 
> > > Don't quote that. It looks bad that the left-side is unquoted and
> > > right side is quoted.
> > 
> > it's a string, what's the problem?
> 
> It's just a matter of taste. I think it looks better if both sides are
> quoted, or neither is.

Right, it's a matter of taste, so the snippet is fine as-is.

Commenting on code is fine, Just don't present your taste as if it is
the only right thing to do.


-- 
Fabian Groffen
Gentoo on a different level

[gentoo-dev] Re: Re: RFC: leechcraft.eclass

[Steven J Long]

Ulrich Mueller wrote:

> But both sides of [[ ]] aren't symmetric, in the first place:
> 
> # When the == and != operators are used, the string to the right of
> # the operator is considered a pattern and matched according to the
> # rules described below under Pattern Matching.
> 
> So there's almost never a reason for quoting the left-hand side, while
> on the right-hand side quotes will suppress pattern matching.
> 
> (Of course, in the example above it doesn't matter, because the string
> doesn't contain any special characters.)
> 
Indeed; minor addition: the above applies to [[ foo = bar* ]] as well,
ie you don't have to use == (and most BASH scripters don't.) It's the fact 
that you're in [[ which triggers fnmatch() behaviour.

-- 
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)