Arch Linux

brofi

Member

Registered: 2015-01-05

Posts: 2

[SOLVED] review xmobar-alsa

Hi,

I'm planning on submitting the following package, which compiles xmobar with the "with_alsa" option to enable the volume plugin:

PKGBUILD xmobar-alsa

For successful compilation the following two packages are required:

PKGBUILD haskell-alsa-mixer
haskell-alsa-mixer.install

PKGBUILD haskell-alsa-core
haskell-alsa-core.install

I'd be glad to get some general feedback on these PKGBUILDs

Last edited by brofi (2016-04-06 19:34:56)

x33a

Forum Fellow

Registered: 2009-08-15

Posts: 4,587

Re: [SOLVED] review xmobar-alsa

Moving to AUR issues.

Scimmia

Fellow

Registered: 2012-09-01

Posts: 11,544

Re: [SOLVED] review xmobar-alsa

I don't know much about haskell packages specifically, so I'll just make general comments about PKGBUILDs.

xmobar-alsa:
You need to change the pkgname
On the license, custom:BSD3 is generally just referred to as 'BSD'.
You're missing a linebreak before the options array.
The options array is unnecessary. 'strip' is part of the default options, there's no reason to have it here.
This should likely 'provide' xmobar
When you're using variables you don't control, like $srcdir and $pkgdir, you should quote them. You don't know if they may contain spaces.

haskell-alsa-mixer:
Why make a _licensefile variable? Seems useless to me.
Same quoting issue as pointed out in xmobar-alsa
Functions are guaranteed to start in $srcdir, you don't need to include that in the 'cd' commands. You can if you want, it just isn't necessary.

haskell-alsa-core:
Same comments as haskell-alsa-mixer

brofi

Member

Registered: 2015-01-05

Posts: 2

Re: [SOLVED] review xmobar-alsa

Thanks for your feedback, I fixed all the issues you pointed out.