On Saturday, 7 January 2017 14:29:58 CET Doug Newgard wrote: > Alright, I've looked at the first one. There's a whole lot of > simplification you could do, and a few actual problems. > > Problems: > > You need to rename the source file. It's just "v1.2.0.tar.gz" which is > very generic and could conflict with other packages.
This is what github creates when you push a tag, I have no choice in the matter. Why would there be a conflict? This is only used for makepkg and any package that is created is always compiled in its own dir. The PKGBUILD file doesn’t have a project name in front of it either, ensuring its all done in its own dir... What am I missing? > Hardcoding the > version here also makes no sense when you could just use $pkgver and only > have to update it in one place when you update. Hmm? I did exactlyt that. The pkver is set once and used everywhere... Maybe you looked at a ‘-git’ package? > The install file does WAY, WAY more than it should. The only thing I see > there that should be happening is creating the user and group, everything > else should be removed. Maybe you are assuming that a ‘make install’ will be able to do everything we need. Unfortunately thats not the case... And I don’t want to change the upstream automake files. So individual install lines are needed. > make -j$(nproc) is bad. The user sets makeflags in makepkg.conf, you > should not be overriding that unless it's necessary. ACK. > Simplifications: > > Functions are guaranteed to start in $srcdir, you don't need to include > that in the cd commands. ACK > All of the "msg2" lines are useless. ACK > In the package function, you cd to $srcdir/bitcoinclassic-$pkgver, but > then you still include that in every install command. It's redundant and > unnecessary. ACK > The cp then desktop-file-install for the .desktop file is really > unnecessary when you could just install it like everything else. ACK > Some of the install commands could be combined. If you're not renaming the > file, you can just specify a target dir with -t and install all files to > that dir at once. > ACK > For renaming, just file a merge request on the right hand side of the page > to merge one package into the other. will do :) > Doug Thanks Doug! -- Tom Zander Blog: https://zander.github.io Vlog: https://vimeo.com/channels/tomscryptochannel
