[gentoo-dev] Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[gentoo-dev] Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[Torsten Veller]

* "Patrick Lauer (patrick)" <patrick@gentoo.org>:
> file : http://sources.gentoo.org/viewcvs.py/gentoo-x86/app-forensics/memdump/memdump-1.0.1.ebuild?rev=1.1&view=markup
> plain: http://sources.gentoo.org/viewcvs.py/gentoo-x86/app-forensics/memdump/memdump-1.0.1.ebuild?rev=1.1&content-type=text/plain
> 
> Index: memdump-1.0.1.ebuild
> ===================================================================
> # Copyright 1999-2009 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Header: /var/cvsroot/gentoo-x86/app-forensics/memdump/memdump-1.0.1.ebuild,v 1.1 2009/01/04 23:45:43 patrick Exp $
> 
> DESCRIPTION="Simple memory dumper for UNIX-Like systems"
> HOMEPAGE="http://www.porcupine.org/forensics"
> SRC_URI="http://www.porcupine.org/forensics/${PN}-1.01.tar.gz"
                                                    ~~~~
> LICENSE="IBM"
> SLOT="0"
> KEYWORDS="~amd64 ~ppc ~x86"
> DEPEND="sys-apps/sed
> 	sys-apps/grep"
> RDEPEND="virtual/libc"
  ^^^^^^^^^^^^^^^^^^^^^^

> IUSE=""
> 
> 
> S=${WORKDIR}/${PN}-1.01
                     ~~~~
> src_compile() {
> 	cd ${S}/memdump-1.01
          "    "        ~~~~
> 	emake XFLAGS="${CFLAGS}" OPT= DEBUG= || die
> }
> 
> src_test() {
> 	if has userpriv ${FEATURES};
                        ~~~~~~~~~~~
		not in pms afair
> 	then
> 		einfo "Cannot test with FEATURES=userpriv"
> 	elif [ -x /bin/wc ];
> 	then
> 		einfo "testing"
> 		if [ "`./memdump -s 344 | wc -c`" = "344" ];
> 		then
> 			einfo "passed test"
> 		else
> 			die "failed test"
> 		fi
> 	fi
> }
> 
> src_install() {
> 	dosbin memdump
                       || die
> 	dodoc README LICENSE
> 	doman memdump.1
> }

Re: [gentoo-dev] Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[Nirbheek Chauhan]

On Mon, Jan 5, 2009 at 5:37 AM, Torsten Veller <ml-en@veller.net> wrote:
> * "Patrick Lauer (patrick)" <patrick@gentoo.org>:
>> file : http://sources.gentoo.org/viewcvs.py/gentoo-x86/app-forensics/memdump/memdump-1.0.1.ebuild?rev=1.1&view=markup
>> plain: http://sources.gentoo.org/viewcvs.py/gentoo-x86/app-forensics/memdump/memdump-1.0.1.ebuild?rev=1.1&content-type=text/plain
>>
>> Index: memdump-1.0.1.ebuild
>> ===================================================================
>> # Copyright 1999-2009 Gentoo Foundation
>> # Distributed under the terms of the GNU General Public License v2
>> # $Header: /var/cvsroot/gentoo-x86/app-forensics/memdump/memdump-1.0.1.ebuild,v 1.1 2009/01/04 23:45:43 patrick Exp $
>>
>> DESCRIPTION="Simple memory dumper for UNIX-Like systems"
>> HOMEPAGE="http://www.porcupine.org/forensics"
>> SRC_URI="http://www.porcupine.org/forensics/${PN}-1.01.tar.gz"
>                                                    ~~~~

Missing MY_PV I presume?

>> LICENSE="IBM"
>> SLOT="0"
>> KEYWORDS="~amd64 ~ppc ~x86"
>> DEPEND="sys-apps/sed
>>       sys-apps/grep"
>> RDEPEND="virtual/libc"
>  ^^^^^^^^^^^^^^^^^^^^^^

Useless deps which are already in @system?

>
>> IUSE=""
>>
>>
>> S=${WORKDIR}/${PN}-1.01
>                     ~~~~

Quotes and ${MY_PV} missing?

>> src_compile() {
>>       cd ${S}/memdump-1.01
>          "    "        ~~~~

Quotes, and ${PN}-${MY_PV} ?
Although, should this even be required since ${S} is already set correctly?
Does the tarball have a folder inside a folder?

>>               einfo "testing"
>>               if [ "`./memdump -s 344 | wc -c`" = "344" ];
>>               then
>>                       einfo "passed test"
>>               else
>>                       die "failed test"
>>               fi

Here, why not use

einfo "testing"
[ "`./memdump -s 344 | wc -c`" = "344" ] || die "failed test"
einfo "passed test"


-- 
~Nirbheek Chauhan

list purpose. was: Re: [gentoo-dev] Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[Daniel Black]

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


I'm not sure I want to see this list being a QA list for commits.

If there is a commit that raises an interesting question for everyone sure put 
it here.

Otherwise please take up QA faults with the author or devrel if you think they 
are consistantly under-standard.

just my 2c.

-- 
Daniel Black <dragonheart@gentoo.org>
Gentoo Foundation

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: list purpose. was: Re: [gentoo-dev] Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[Mark Loeser]

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

Daniel Black <dragonheart@gentoo.org> said:
> I'm not sure I want to see this list being a QA list for commits.
> 
> If there is a commit that raises an interesting question for everyone sure put 
> it here.
> 
> Otherwise please take up QA faults with the author or devrel if you think they 
> are consistantly under-standard.

I'm pretty sure this has come up before...  The developer list is the
best place for us to do peer review, since it not only helps the person
that is being reviewed, but also helps other developers at the same
time since they can see issues and how to correct them.

My 2c would be that if you don't want to see them to set up a filter.

-- 
Mark Loeser
email         -   halcy0n AT gentoo DOT org
email         -   mark AT halcy0n DOT com
web           -   http://www.halcy0n.com

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

[gentoo-dev] Re: list purpose. was: Re: Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[Duncan]

Mark Loeser <halcy0n@gentoo.org> posted
20090105162921.GA15481@aerie.halcy0n.com, excerpted below, on  Mon, 05 Jan
2009 11:29:21 -0500:

> Daniel Black <dragonheart@gentoo.org> said:
>> I'm not sure I want to see this list being a QA list for commits.
>> 
>> If there is a commit that raises an interesting question for everyone
>> sure put it here.
>> 
>> Otherwise please take up QA faults with the author or devrel if you
>> think they are consistantly under-standard.
> 
> I'm pretty sure this has come up before...  The developer list is the
> best place for us to do peer review, since it not only helps the person
> that is being reviewed, but also helps other developers at the same time
> since they can see issues and how to correct them.
> 
> My 2c would be that if you don't want to see them to set up a filter.

Seconded.  This is specifically a technical list (project is otherwise), 
and I'm not a dev but learn a lot from those posts, which are after all 
technical and you couldn't get more gentoo-dev topic-wise.

-- 
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

Re: [gentoo-dev] Re: gentoo-x86 commit in app-forensics/memdump: memdump-1.0.1.ebuild ChangeLog

[Peter Volkov]

Daniel, answering you in this mail... Yes, probably sometimes we have to
discuss basic things in this mailing list but since developers do this
basic errors we have to. Technical problems in our portage tree are
perfectly valid for this mailing list.

В Пнд, 05/01/2009 в 13:55 +0530, Nirbheek Chauhan пишет:
> >> SRC_URI="http://www.porcupine.org/forensics/${PN}-1.01.tar.gz"                                                    ~~~~
> 
> Missing MY_PV I presume?

Hardcoded version makes version bumps harder. I'm not sure why ebuild
was versioned differently from tarbal but yes, in this case it's better
to use versionator eclass define PV.

> >> RDEPEND="virtual/libc"
> >  ^^^^^^^^^^^^^^^^^^^^^^
> 
> Useless deps which are already in @system?

Dependency on libc is completely useless since every usable system have
C library.

> >> S=${WORKDIR}/${PN}-1.01
> >                     ~~~~
> 
> Quotes and ${MY_PV} missing?

You don't need quotes in assignments. It hardcoded version again.

> >> src_compile() {
> >>       cd ${S}/memdump-1.01
> >          "    "        ~~~~
> 
> Quotes, and ${PN}-${MY_PV} ?
> Although, should this even be required since ${S} is already set correctly?

src_compile initial working directory is S so this line could (and
should) be just dropped.

> >>               einfo "testing"
> >>               if [ "`./memdump -s 344 | wc -c`" = "344" ];
> >>               then
> >>                       einfo "passed test"
> >>               else
> >>                       die "failed test"
> >>               fi
> 
> Here, why not use
> 
> einfo "testing"
> [ "`./memdump -s 344 | wc -c`" = "344" ] || die "failed test"
> einfo "passed test"

Yup. It's shorter and does same thing.

-- 
Peter.