Date: Thu, 3 Sep 2015 10:51:27 +0200
From: =?iso-8859-1?Q?S=F6ren?= Tempel <soeren@soeren-tempel.net>
To: alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports] [PATCH] Adding Googles Logging Library
Message-ID: <20150903085127.GA2435@calcium.lan>
References: <1441135929-24200-1-git-send-email-mvenezia@gmail.com>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <1441135929-24200-1-git-send-email-mvenezia@gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)

On 01.09.15, Michael Venezia wrote:
> ---
>  main/google-glog/APKBUILD | 44 +++++++++++++++++++++++++++++++++++++++++=
+++
>  1 file changed, 44 insertions(+)
>  create mode 100644 main/google-glog/APKBUILD

You committed this to main/ new packages should stay in testing/ for
a while. Furthermore, the commit message should be "testing/google-glog:
new aport". In addition to that: Why did you call the package
google-glog instead of simply calling it glog?

On 01.09.15, Michael Venezia wrote:
> +arch=3D"x86_64"

I am pretty sure that glog works on x86 and possibly arm too. Please
adjust this accordingly. If it works on arm and x86 too you can simply
use arch=3D"all".

On 01.09.15, Michael Venezia wrote:
> +license=3D"custom:BSD3"

This notation is really strange, either use custom and install the
license file into /usr/share/licenses or use license=3D"BSD". I would
prefer the former.

On 01.09.15, Michael Venezia wrote:
> +depends_dev=3D"make gcc"

Two things: a) depends_dev is supposed to be used for -dev packages only,
those two package should theoretically be makedepends b) You don't need
to explicitly depend on make and gcc since they are both a part of
alpine-sdk and are thus installed by abuild(1) by default.

On 01.09.15, Michael Venezia wrote:
> +package() {
> +	cd "$_builddir"
> +  cd "$srcdir/glog-${pkgver}"
> +  make DESTDIR=3D"$pkgdir" install
> +
> +	find "$pkgdir" -name '*.la' -delete
> +}

As you can see the indention is completely broken here. Besides you
don't need to remove *.la files manually since abuild(1) automatically
removes them unless you enable the libtool option. Also why do you cd
into the undefined "$_builddir" here? That doesn't make any sense at
all.

S=F6ren.


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---