[aur-general] Package review bitcoin-classic

[Doug Newgard]

On Sat, 07 Jan 2017 16:41:42 +0100
Tom Zander <tomz at freedommail.ch> wrote:

> Hi, 
> 
> I created some AUR packages after various people requested it (I'm the 
> upstream release manager) and since I'm rather new to packaging in Arch, I 
> would love to have some feedback. Arch packaging is consederably easier than 
> debian, I might add ;)
> 
> There are 4, almost identical, versions.
> 
> https://aur.archlinux.org/packages/bitcoin-classic/
> https://aur.archlinux.org/packages/bitcoin-classic-daemon/
> https://aur.archlinux.org/packages/bitcoin-classic-git/
> https://aur.archlinux.org/packages/bitcoind-classic-git/
> 
> I inherited some of those, hence the slight difference in naming. Not sure 
> if renaming is possible and if the bitcoind- one may be more easy to find 
> under a different name.
> 
> Any feedback welcome, and naturally I'd love it if those packages would be 
> able to reach the community repo, but maybe they need a little more time to 
> mature, I'm not sure.
> 
> Thanks!

Alright, I've looked at the first one. There's a whole lot of simplification
you could do, and a few actual problems.

Problems:

You need to rename the source file. It's just "v1.2.0.tar.gz" which is very
generic and could conflict with other packages. Hardcoding the version here
also makes no sense when you could just use $pkgver and only have to update it
in one place when you update.

The install file does WAY, WAY more than it should. The only thing I see there
that should be happening is creating the user and group, everything else should
be removed.

make -j$(nproc) is bad. The user sets makeflags in makepkg.conf, you should not
be overriding that unless it's necessary.

Simplifications:

Functions are guaranteed to start in $srcdir, you don't need to include that in
the cd commands.

All of the "msg2" lines are useless.

In the package function, you cd to $srcdir/bitcoinclassic-$pkgver, but then you
still include that in every install command. It's redundant and unnecessary.

The cp then desktop-file-install for the .desktop file is really unnecessary
when you could just install it like everything else.

Some of the install commands could be combined. If you're not renaming the
file, you can just specify a target dir with -t and install all files to that
dir at once.


For renaming, just file a merge request on the right hand side of the page to
merge one package into the other.

Doug