[aur-general] Proofreading request

[Clément Junca]

2013/8/27 Dave Reisner <d at falconindy.com>

> 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 at gmail.com
> > > To: aur-general at archlinux.org
> > > Subject: Re: [aur-general] Proofreading request
> > >
> > > 2013/8/27 Taylor Lookabaugh <jesus.christ.i.love at 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
>


Thanks a lot to all of you for theses corrections. I learned a lot.

If I understand correctly putting the configuration file directly to /etc/
is fine as long as I declare it in the backup array.

A few notes:
* The 'backup' field is clearly explained in the wiki (or I haven't found
it).
* The "|| exit 1" comes from the PKGBUID of eject-unlock (not saying that
to point a responsible be to allow this improvement to other packages).
* /usr/share/pacman/PKGBUILD-git.proto use an explicit "git clone" and
seems older than the example in the wiki.