From mboxrd@z Thu Jan 1 00:00:00 1970
Received: from pigeon.gentoo.org ([208.92.234.80] helo=lists.gentoo.org)
by finch.gentoo.org with esmtp (Exim 4.60)
(envelope-from <gentoo-dev+bounces-44904-garchives=archives.gentoo.org@lists.gentoo.org>)
id 1Q2Ahz-0001ez-Eq
for garchives@archives.gentoo.org; Tue, 22 Mar 2011 23:09:53 +0000
Received: from pigeon.gentoo.org (localhost [127.0.0.1])
by pigeon.gentoo.org (Postfix) with SMTP id B390F1C051;
Tue, 22 Mar 2011 23:09:42 +0000 (UTC)
Received: from mail-fx0-f53.google.com (mail-fx0-f53.google.com [209.85.161.53])
by pigeon.gentoo.org (Postfix) with ESMTP id 76BBF1C001
for <gentoo-dev@lists.gentoo.org>; Tue, 22 Mar 2011 23:09:14 +0000 (UTC)
Received: by fxm11 with SMTP id 11so9187167fxm.40
for <gentoo-dev@lists.gentoo.org>; Tue, 22 Mar 2011 16:09:13 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=gamma;
h=domainkey-signature:mime-version:sender:in-reply-to:references:from
:date:x-google-sender-auth:message-id:subject:to:cc:content-type
:content-transfer-encoding;
bh=h9eImniBP7iJDoRTDQoerpdutxcSqFQKOSNe/q4eQ64=;
b=NjVoNqrYQIT+QIEylSOt2k/MHv4Mq67aaz/tGwUWoxhkmtAMnJTrEuYlTdZAlZnYNm
pmCkMGB6nsroKvTjyQdiuk0HRj/FTZbB0Am4KTF2iEpRrA3hsGy03yZZF5Klam5FOAWA
/37IGrQVu7DsSDuPYb/5JMxF3DJUQEJtAk1vs=
DomainKey-Signature: a=rsa-sha1; c=nofws;
d=gmail.com; s=gamma;
h=mime-version:sender:in-reply-to:references:from:date
:x-google-sender-auth:message-id:subject:to:cc:content-type
:content-transfer-encoding;
b=nYJc7BbszJWjuzilZM0ytxQiG02uh8ZWmSlTLsSZvnoHW8HJkPsnjzPu2tlEvhIn93
cFLnB7jtI9LbDND/jk1waz0+27mpgRY9PQb+vfbw2TA3Ao2fq1p9Ca5kcGYEbQG6qFBf
/bEqwYxD+wljY7hrKvzimNcFiKyYPLlv5KDgE=
Received: by 10.223.15.141 with SMTP id k13mr2229648faa.30.1300835353180; Tue,
22 Mar 2011 16:09:13 -0700 (PDT)
Precedence: bulk
List-Post: <mailto:gentoo-dev@lists.gentoo.org>
List-Help: <mailto:gentoo-dev+help@lists.gentoo.org>
List-Unsubscribe: <mailto:gentoo-dev+unsubscribe@lists.gentoo.org>
List-Subscribe: <mailto:gentoo-dev+subscribe@lists.gentoo.org>
List-Id: Gentoo Linux mail <gentoo-dev.gentoo.org>
X-BeenThere: gentoo-dev@lists.gentoo.org
Reply-to: gentoo-dev@lists.gentoo.org
MIME-Version: 1.0
Sender: vapierfilter@gmail.com
Received: by 10.223.5.15 with HTTP; Tue, 22 Mar 2011 16:08:53 -0700 (PDT)
In-Reply-To: <4D8924CD.8000404@gentoo.org>
References: <4D890F8D.4090706@gentoo.org> <AANLkTiky_tfyUE=t-PcWPa+Gu9aMgyacqmR09XTfmTb+@mail.gmail.com>
<4D8924CD.8000404@gentoo.org>
From: Mike Frysinger <vapier@gentoo.org>
Date: Tue, 22 Mar 2011 19:08:53 -0400
X-Google-Sender-Auth: tVOUSRnW2z0sYiKS7VXBw_B_zzU
Message-ID: <AANLkTingeQcC+ZLw9G4W37-9pQ7rdAkmFsaRHDUcRvsj@mail.gmail.com>
Subject: Re: [gentoo-dev] git-2.eclass final review
To: gentoo-dev@lists.gentoo.org
Cc: =?ISO-8859-2?Q?Tom=E1=B9_Chv=E1tal?= <scarabeus@gentoo.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
X-Archives-Salt:
X-Archives-Hash: b7b8e17c18915e0489d09bc8afb58761
2011/3/22 Tom=C3=A1=C5=A1 Chv=C3=A1tal:
> Dne 22.3.2011 22:26, Mike Frysinger napsal(a):
>>> # @BLURB: This eclass provides functions for fetch and unpack git repos=
itories
>>
>> fetching/unpacking
>
> Yarp fixed.
well, the fix broke the blurb. it has to be on one line.
# @BLURB: foo
> EGIT_BRANCH=3D${x:-${EGIT_BRANCH:=3D${EGIT_MASTER}}}
> EGIT_COMMIT=3D${x:-${EGIT_COMMIT:=3D${EGIT_BRANCH}}}
doesnt make much sense to use :=3D ... it should be :-
> [[ "$#" -ne 1 ]] && die "${FUNCNAME}: requires 1 argument (path)"
quoting doesnt make much sense ... -ne compares an int, not a string
also, the error msg is a bit vague. it should say "... requires exactly 1 =
..."
> pushd "${1}" &> /dev/null
> popd &> /dev/null
do you really want to silence errors ? normally people only send
stdout to /dev/null because of the echoed dirlist.
seems to come up in every func
> git submodule init || die "${FUNCNAME}: git submodule initialisation fail=
ed"
> git submodule sync "" die "${FUNCNAME}: git submodule sync failed"
> git submodule update || die "${FUNCNAME}: git submodule update failed"
the die strings are abit redundant ... when it fails, the output will
show "git submodule init || die", so people can easily figure out "the
git submodule init cmd failed"
> die "\"${EGIT_BOOTSTRAP}\" is not executable."
i find periods in die messages which are a single sentence to be
noise. but maybe that's just me.
> if [[ "${EGIT_COMMIT}" !=3D "${EGIT_BRANCH}" ]]; then
quoting doesnt matter here since you're using [[...]]
seems to come up in a bunch of places
> local save_sandbox_write=3D${SANDBOX_WRITE}
> if [[ ! -d "${EGIT_STORE_DIR}" ]] ; then
> ...
> SANDBOX_WRITE=3D${save_sandbox_write}
> fi
might as well have the save done inside the if statement since that's
the only place it's used
> rsync -rlpgo . "${EGIT_SOURCEDIR}" \
this means you need to have DEPEND=3D"net-misc/rsync". why not just use
`cp -pPR` instead ? i vaguely recall rsync being slower than a
straight cp too ... not much point of doing a rsync when the vast
majority of the time (all the time?) the destination is empty.
> git-2_initial_clone()
> git-2_update_repo()
shouldnt EGIT_REPO_URI_SELECTED be set to "" at the start ?
> for x in $(git branch |grep -v "* ${EGIT_MASTER}" |tr '\n' ' '); do
missing space after each pipe
> if [[ -n ${EGIT_BOOTSTRAP} ]] ; then
> if [[ -f ${EGIT_BOOTSTRAP} ]]; then
seems inconsistent in the whole file ... personally, i prefer the
space before the semicolon in if statements
-mike