On Tue, Aug 27, 2013 at 10:34:56AM -0500, Doug Newgard wrote:
----------------------------------------
Date: Tue, 27 Aug 2013 10:04:37 +0200 From: cju.arch@gmail.com To: aur-general@archlinux.org Subject: Re: [aur-general] Proofreading request
2013/8/27 Taylor Lookabaugh <jesus.christ.i.love@gmail.com>
On 08/27/13 00:35, Clément Junca wrote:
Yes, you're right. Sorry. Here is the good one. You haven't attached anything to this mail.
PS: make sure you reply below the quotes in a mailing list, easier to read top to bottom.
That's strange, I see the tar.gz file in my sent mail. Here are the files from the archive.
My notes:
1. Get rid of all of the empty variables (groups, provides, etc) 2. Definitely add the license file to the source array. 3. The cd .. at the end of the pkgver function is useless. 4. Applying the patch should be done in a prepare() function, you don't need a build() function at all in this case. 5. You don't need || exit 1. The functions are called in a way so it will already exit if there are errors. 6. install -D will make the dirs it needs, you don't need to make them yourself with mkdir -p. 7. The comment in the pkgver function doesn't match what it's doing, it's not using a tag. 8. If you do install the default config file, you should add it to the backup array so that pacman doesn't overwrite it every time you upgrade.
I will disagree with the previous posters on a couple of things.
1. There's nothing wrong with using ../../LICENSE as long as you know what dir you're in.
Yes. There is. You don't, and you can't know what directories are above you in this case. This PKGBUILD will fail when [[ -n $BUILDDIR ]] is true. More pertinent, not adding the LICENSE file to the source array means that 'makepkg -S' doesn't include this file. That the file is still included is a sign of a manually crafted source tarball and a huge red flag.
2. There is nothing wrong with cd-ing directly to $_gitname, although I prefer $srcdir/$_gitname myself