On Tue, 14 Oct 2008, Ronald van Haren wrote:

On Tue, Oct 14, 2008 at 12:15 PM, Sergej Pupykin wrote:

...

List of packages: http://aur.archlinux.org/packages.php?O=0&L=0&C=0&K=xhemi&SeB=m&SB=n&SO=a

On Tue, Oct 14, 2008 at 2:12 PM, Sergej Pupykin wrote:

...

Hi,

I want to sponsor Edward Tjörnhammar(xhemi). His packages have small issues such as backup lines or missing ' || return 1', but they are good.

So I ask to start discussion.

...

I recently contributed a package 'inn' which I intend to continue working on. The package was added to [community] so I will continue with as inn-orig in AUR, user xhemi. If I were to become TU I believe that I can help to improve any number of packages in need of love but would continue with 'inn'. My sponsor is Sergej Pupykin.

Sorry to say, but I found quite a few things from a very quick look:

- errors in licenses. This is an array and you need to install custom licenses in /usr/share/licenses/$pkgname - you need to include patches and such in the source array - Do not introduce new variables into your PKGBUILD build scripts, unless the package cannot be built without doing so. Further some of them are already defined by a variable so why creating a new one? I really have trouble reading it. - what is up with the use of pushd and popd? There is clearly no use in using those.

What else can I say...I still think 8 packages is not enough as I already indicated in a previous thread.

Ronald

Hello again! The unincluded licenses, sources and backup files in their respective arrays are something which I should have given more than a second thought and I agree that they need to be fixed. There is, however, likely other things which I might have missed. About the criticism surrounding variable use and pushd/popd I can only regard as irrelevant, but please prove me wrong. The variables are used in a completely consistent manner and pushd/popd are shell builtins just like cd, i.e. does not rely on extra deps. This is just a matter of preference, instead of 'cd' and 'cd -', just like wether to use srcdir or _S. If this is important then there is, imho, only one place where this should be rigorously checked for and that would be in a pre-commit hook for the SCM tree for supported TU packages. It would not be my intention to move any more of my packages to community, since they have very low voting scores. As you obviously understand by now I was assuming that there were alot of orphaned packages. Regards -- Edward Tjörnhammar

On Tue, Oct 14, 2008 at 9:28 AM, Aaron Schaefer wrote:

On Tue, Oct 14, 2008 at 9:10 AM, Edward Tjörnhammar wrote:

...

On Tue, 14 Oct 2008, Ronald van Haren wrote:

...

Sorry to say, but I found quite a few things from a very quick look:

- errors in licenses. This is an array and you need to install custom licenses in /usr/share/licenses/$pkgname - you need to include patches and such in the source array - Do not introduce new variables into your PKGBUILD build scripts, unless the package cannot be built without doing so. Further some of them are already defined by a variable so why creating a new one? I really have trouble reading it. - what is up with the use of pushd and popd? There is clearly no use in using those.

What else can I say...I still think 8 packages is not enough as I already indicated in a previous thread.

Ronald

Hello again!

The unincluded licenses, sources and backup files in their respective arrays are something which I should have given more than a second thought and I agree that they need to be fixed. There is, however, likely other things which I might have missed. About the criticism surrounding variable use and pushd/popd I can only regard as irrelevant, but please prove me wrong. The variables are used in a completely consistent manner and pushd/popd are shell builtins just like cd, i.e. does not rely on extra deps. This is just a matter of preference, instead of 'cd' and 'cd -', just like wether to use srcdir or _S. If this is important then there is, imho, only one place where this should be rigorously checked for and that would be in a pre-commit hook for the SCM tree for supported TU packages. It would not be my intention to move any more of my packages to community, since they have very low voting scores. As you obviously understand by now I was assuming that there were alot of orphaned packages.

Regards -- Edward Tjörnhammar

The variables are distracting and make it hard to read, not to mention some of them are already defined for you. The Arch Packaging Guidelines talk about that specifically:

"Do not introduce new variables into your PKGBUILD build scripts, unless the package cannot be built without doing so, as these could possibly conflict with variables used in makepkg itself. If a new variable is absolutely required, prefix the variable name with an underscore (_)"

Basically, they are not necessary, and don't follow the same standards that everyone else use. There is no easy way to check for proper use of custom variables with a commit hook because how can you programatically determine whether or not they are necessary?

I feel pretty much the same way about pushd/popd...while yes, they are builtins, they do make your PKGBUILDs harder to follow (explicitly stating a directory is much more clear than trying to remember a stack of directories you pushed previously), and in many cases you don't push more than a single directory anyway and/or it's not necessary to change back to the base directory, so why complicate things?

On top of everything that Ronald mentioned:

- you should not have the Maintainer tag in there, as those specify the TU responsible for maintaining the binary version of the package. - SourceForge URLs should not specify a mirror but should use http://downloads.sourceforge.net/sourceforge/... - try to have due diligence in tracking down the proper license for packages; ccxstream, for instance, you have listed as "unknown" when two clicks to the project's main page shows it is listed as GPL - indention, spacing, and consistency/cleanliness do matter, especially if you're trying to get other TUs to look at your PKGBUILDs and vote you in. Take a look at the axis2c PKGBUILD (http://is.gd/42ZH) and notice the spacing, and also you have "|| return 1" twice on the end of one line - rather than using heredoc statements to create files within the PKGBUILD, it would be cleaner to include separate files in the source array

Overall, I'd recommend reading up on the Packaging Guidelines, using tools like namcap, take a look at PKGBUILDs from existing TUs and keep working at it. We don't expect for an applicant's PKGBUILDs to be absolutely perfect, but realistically, they should be close if that's all we have to judge you by.

Aaron "ElasticDog" Schaefer --

All good info and advice. It should be followed whenever possible (for the most part anyways). Even better would be to look at a bunch of PKGBUILDS and see how others do stuff. Grep would be your friend in this endeavor. Look for BOTH good and bad examples. Best would be for you to develop the ability to see immediately what was a good example and a bad example and be able to say why.