[aur-general] Package review bitcoin-classic

[Bruno Pagani]

Le 07/01/2017 à 16:41, Tom Zander a écrit :

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

So, some days later… Here is my review, which is to be added to Doug
comments. Some (most?) are styles choice, so you’re free to follow them
or not. ;) Given the high level of similarity of your 4 packages, all
comments apply to most of them. That’s also a hint for something bigger:
you could probably setup a split PKGBUILD, and have bitcoin-classic to
only pack the GUI and depends on bitcoin-daemon-classic for the rest
(and same with -git variants). See transmission[0] for example, and
don’t hesitate to ask for help if you want to go that way. ;)

– Please be consistent in your use of vars: either ${var} or $var, but
don’t mixed them. I think the only instance is
${pkgname}-${pkgver}.tar.gz in source array because your borrowed it
from my comment. ;)

– I think you could declare a _pkgname=bitcoinclassic var and use it in
several places. I like my package to make use of var has much has
possible to make any change easy:

|url="https://$_pkgname.com" ||source=(${pkgname}-${pkgver}.tar.gz::"https://github.com/$_pkgname/$_pkgname/archive/v$pkgver.tar.gz"
||cd "$_pkgname-$pkgver" And so on… |

– In the same idea, you could declare a _pkgorig=bitcoin (wasn’t
inspired at var name) and use it almost elsewhere
(provides/conflicts,package() function).

– It’s advised to split ./configure option on multiple lines for
increased {read,edit}ability. Example (bitcoin-classic):

|./configure \ --prefix=/usr \ --with-incompatible-bdb \ --with-gui=qt5 \
--enable-hardening \ --enable-reduce-exports \ --disable-gui-tests \
--disable-maintainer-mode|

– In your -git packages,

|cd "$srcdir/bitcoinclassic" |

should be: cd "bitcoinclassic"

That’d be all for this first pass, I can have another look if you want
once you’ll have fixed Doug concerns and implemented the changes you
want among the above ones. ;)

Cheers,
Bruno

[0]
https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/transmission
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 520 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20170111/efeda173/attachment.asc>