User-Agent: Cyrus-JMAP/3.3.0-dev0-504-g204cd6f-fm-20200527.002-g204cd6f2
Mime-Version: 1.0
Message-Id: <9aecb18b-c0f5-422d-89b4-bc156b654863@www.fastmail.com>
In-Reply-To: <20200530081121.83fb92a61b46617ca6284934@gmail.com>
References: <20200530021355.20187-1-m@milesalan.com>
 <20200530081121.83fb92a61b46617ca6284934@gmail.com>
Date: Sat, 30 May 2020 09:51:24 -0500
From: "Miles Alan" <m@milesalan.com>
To: "Konstantin Kulikov" <k.kulikov2@gmail.com>
Cc: ~alpine/aports@lists.alpinelinux.org
Subject: Re: [PATCH] testing/lf: new aport
Content-Type: text/plain

On Sat, May 30, 2020, at 12:11 AM, Konstantin Kulikov wrote:
> Many devs ignore aports ML, consider sending merge request at 
> https://gitlab.alpinelinux.org/alpine/aports
> 
> Review comments inline below.
> 
> > https://github.com/gokcehan/lf
> > Terminal filemanager written in Go with vim-style keybindings
> > ---
> >  testing/lf/APKBUILD | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> >  create mode 100644 testing/lf/APKBUILD
> > 
> > diff --git a/testing/lf/APKBUILD b/testing/lf/APKBUILD
> > new file mode 100644
> > index 00000000..f79c70ae
> > --- /dev/null
> > +++ b/testing/lf/APKBUILD
> > @@ -0,0 +1,36 @@
> > +# Maintainer: Miles Alan <m@milesalan.com>
> > +pkgname=lf
> > +pkgver=14
> > +pkgrel=0
> > +pkgdesc="Terminal filemanager written in Go with vim-style keybindings"
> > +url="https://github.com/gokcehan/lf"
> > +arch="all"
> > +license="MIT"
> > +makedepends="go"
> > +subpackages="$pkgname-doc"
> > +source="$pkgname-$pkgver.tar.gz::https://github.com/gokcehan/lf/archive/r$pkgver.tar.gz"
> > +builddir="$srcdir/src/github.com/gokcehan/$pkgname"
> 
> builddir is unneccesary with go modules. 
> 
> > +options="chmod-clean"
> 
> Add "net" to options as well. This may become a requirement in future.
> 
> Also export a few variables so go doesn't put stuff in /home or /tmp like this:
> 
> export GOPATH="$srcdir/go"
> export GOCACHE="$srcdir/go-build"
> export GOTMPDIR="$srcdir"
> 
> > +
> > +prepare() {
> > +	mkdir -p ${builddir%/*}
> > +	mv $srcdir/$pkgname-r$pkgver "$builddir"/
> > +	default_prepare
> > +	go mod vendor
> > +}
> 
> If you vendor stuff you need to also pass -mod=vendor to "go build" and 
> "go test",
> because alpine is still on go1.13.
> remove prepare() function and vendoring completely instead, it is 
> unneccesary here.
> 
> > +
> > +build() {
> > +	go build -v -o bin/$pkgname
> > +}
> > +
> > +check() {
> > +	go test
> 
> Prefer "go test ./..." just in case upstream adds more packages to repo 
> in future.
> 
> > +}
> > +
> > +package() {
> > +	install -Dm755 "$builddir"/bin/$pkgname "$pkgdir"/usr/bin/$pkgname
> > +	install -Dm644 LICENSE "$pkgdir"/usr/share/licenses/$pkgname/LICENSE
> 
> Do not install license file, unless package uses "license=custom".
> 
> > +	install -Dm644 lf.1 "$pkgdir"/usr/share/man/man1/lf.1
> > +}
> > +
> > +sha512sums="cd2e268d551a0a83a32563424675455288e09b28d60940143eff4d24cf5b98ad0d7bd9ab100ea5a456e37460c318001ad5b444d252a986e9ddcfff274adf7720  lf-14.tar.gz"
> > -- 
> > 2.26.2
>

Thanks for the feedback - will address and reopen as MR on gitlab.