----------------------------------------

Date: Mon, 10 Feb 2014 00:15:55 +0100 From: okraits@arcor.de To: aur-general@archlinux.org Subject: [aur-general] Code review request: new PKGBUILD "omodoro"

Hello trusty Arch people,

i just created my first PKGBUILD for the little python script "omodoro" i wrote. It would be nice if you guys could do a code review to approve the PKGBUILD i wrote for the AUR.

The PKGBUILD is here:

http://www.okraits.de/upload/omodoro/PKGBUILD

Any feedback/criticism is appreciated!

Greetings,

Oliver

1. Your pkgver function doesn't work. pkgver should not be random, it should increase with each commit to the repo.  2. Don't have it depend on an old, replaced package (python3). You just want 'python'. 3. Please quote paths that contain a variable like $pkgdir. 4. You should append -git to the pkgname when you're pulling from git master. 5. Why bother to define _gitname if it's the same as pkgname? Since you should be changing pkgname, _gitname can make sense or you can just do ${pkgname%-git} and get the same thing without having to use an extra variable. Those are my suggestions, nothing too serious.

Hi On Sun, Feb 9, 2014 at 9:08 PM, David Phillips <dbphillipsnz@gmail.com> wrote:

Related to (4): I've got a PKGBUILD (https://aur.archlinux.org/packages/pa/passgen/PKGBUILD) which points to a git repo, but *not* using a "git://xyz.git" url. Instead, I'm using the format "https://github.com/phillid/passgen/archive/master.zip" - should this package have -git appended?

Yes, it should. The "-git" suffix indicates that the package is built from VCS. "master.zip" source file *also* comes from VCS, from git repo and means "current HEAD of master branch". The main point of VCS package is that the source is moving target and it changes independently from package maintainer. And it differs from 'versioned' package where sources are changed only when maintainer updates PKGBUILD. Using "master.zip" is a bad idea because: - Once master.zip downloaded it is cached in the local directory. If you run makepkg again nothing will be redownloaded even if upstream repository has changed. - Updating local git repository is more effective as it fetches only the latest changes. Thus package rebuild is faster. So what you want for passgen: - rename it to passgen-git - use git repository for sources=() instead of 'master.zip' - add 'git' to makedepends - add proper pkgver() - use 'SKIP' for sha1sum as project head can change at any moment

I'm thinking it doesn't need the -git suffix because it's pulled over HTTP and doesn't need git... What's the convention on this?

Cheers.

On 10/02/2014, Doug Newgard <scimmia22@outlook.com> wrote:

...

----------------------------------------

...

Date: Mon, 10 Feb 2014 00:15:55 +0100 From: okraits@arcor.de To: aur-general@archlinux.org Subject: [aur-general] Code review request: new PKGBUILD "omodoro"

Hello trusty Arch people,

i just created my first PKGBUILD for the little python script "omodoro" i wrote. It would be nice if you guys could do a code review to approve the PKGBUILD i wrote for the AUR.

The PKGBUILD is here:

http://www.okraits.de/upload/omodoro/PKGBUILD

Any feedback/criticism is appreciated!

Greetings,

Oliver

1. Your pkgver function doesn't work. pkgver should not be random, it should increase with each commit to the repo. 2. Don't have it depend on an old, replaced package (python3). You just want 'python'. 3. Please quote paths that contain a variable like $pkgdir. 4. You should append -git to the pkgname when you're pulling from git master. 5. Why bother to define _gitname if it's the same as pkgname? Since you should be changing pkgname, _gitname can make sense or you can just do ${pkgname%-git} and get the same thing without having to use an extra variable.

Those are my suggestions, nothing too serious.

-- David Phillips

Hi Doug, On Sun, Feb 09, 2014 at 06:12:50PM -0600, Doug Newgard wrote:

1. Your pkgver function doesn't work. pkgver should not be random, it should increase with each commit to the repo.

I copied it from some PKGBUILD - obviously from a bad one What about: echo $(git rev-list --count HEAD).$(git rev-parse --short HEAD) This returns: 10.51b537b Or what about this: git log -1 --format="%cd" --date=short | sed 's|-||g' This returns: 20140205 Or what about this combination: git log -1 --format="%cd.g%h" --date=short | sed 's/-/./g' This returns: 2014.02.05.g51b537b I took these examples from the PKGBUILDs with the most votes on the AUR. What is recommended for the pkgver function?

2. Don't have it depend on an old, replaced package (python3). You just want 'python'.

Fixed it.

3. Please quote paths that contain a variable like $pkgdir.

Fixed it.

4. You should append -git to the pkgname when you're pulling from git master.

Fixed it.

5. Why bother to define _gitname if it's the same as pkgname? Since you should be changing pkgname, _gitname can make sense or you can just do ${pkgname%-git} and get the same thing without having to use an extra variable.

Ok, i added "-git" to pkgname, now _gitname makes sense. Thanks for your suggestions. The updated PKGBUILD is again available at http://www.okraits.de/upload/omodoro/PKGBUILD Greetings, Oliver

Hi Justin, thanks for that link - i didn't run into that page on the wiki. So i will use the last git pkgver() example on that page: printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)" -> r1142.a17a017 This is incremental, independent of tags and includes the fingerprint for reference. Greetings, Oliver On Mon, Feb 10, 2014 at 08:43:49PM +1000, Justin Dray wrote:

For the git pkgver functions, you're best off checking the wiki: https://wiki.archlinux.org/index.php/VCS_PKGBUILD_Guidelines

The first one you posted is the only one listed there. The function you are using at the moment will return a random version as it's just the fingerprint of the latest commit; That means it may or may not get seen as an update when people go to build the next version. You need to make sure it is something that makes sense and increments as new commits are made. You should rarely have to deviate from what is on that VCS PKGBUILD guidelines page.