I am being sponsored by dvzrv.
I've been working in the AUR since 2014 as 'Ainola'. I've had a few of my packages adopted into [community], such as gnome-mpv, csound, and qutecsound (the latter two being adopted by dvzrv).
I am also an active contributor to the LibreOffice infrastructure team.
I would like to get valuable tools promoted into [community], such as residualvm or the 'pass' plasmoid (after prodding upstream to have a formal release). Other packages that I do not maintain in the AUR would be ripe for picking as well, such as sc-controller or ttf-lato. I would also work with dvzrv on maintaining pro-audio packages, many of which were abandoned when SpepS left.
On 2018-10-25 08:26:11 (-0600), Brett Cornwall via aur-general wrote:
I am being sponsored by dvzrv.
I hereby ACK that and apologize for the confusion the last time (again).
I would like to get valuable tools promoted into [community], such as residualvm or the 'pass' plasmoid (after prodding upstream to have a formal release). Other packages that I do not maintain in the AUR would be ripe for picking as well, such as sc-controller or ttf-lato. I would also work with dvzrv on maintaining pro-audio packages, many of which were abandoned when SpepS left.
That would be very awesome!
The best of luck to you and let the discussion begin! This seems to be a good month for TU applications.
Best, David
P.S.: As you've just created a new pgp key pair for your address, please make sure to upload the pubkey to the keyservers!
Hi Brett
On 10/25/18 8:22 PM, David Runge wrote:
P.S.: As you've just created a new pgp key pair for your address, please make sure to upload the pubkey to the keyservers!
can you please fix this and make your gpg key available somewhere?
On 10/26/18 08:44pm, Levente Polyak via aur-general wrote:
can you please fix this and make your gpg key available somewhere?
I've pushed 0F8E620A up.
On 10/25/18 at 08:26am, Brett Cornwall via aur-general wrote:
I am being sponsored by dvzrv.
I've been working in the AUR since 2014 as 'Ainola'. I've had a few of my packages adopted into [community], such as gnome-mpv, csound, and qutecsound (the latter two being adopted by dvzrv).
I am also an active contributor to the LibreOffice infrastructure team.
What kind of tasks/roles do you handle for LibreOffice in the infrastructure team?
I would like to get valuable tools promoted into [community], such as residualvm or the 'pass' plasmoid (after prodding upstream to have a formal release). Other packages that I do not maintain in the AUR would be ripe for picking as well, such as sc-controller or ttf-lato. I would also work with dvzrv on maintaining pro-audio packages, many of which were abandoned when SpepS left.
I always like it when applicants want to improve/work on orphan/neglected packages in the repos!
Couldn't find much issues in your packages!
On 10/25/18 10:22pm, Jelle van der Waa wrote:
What kind of tasks/roles do you handle for LibreOffice in the infrastructure team?
I've been working with the team for a few years now. I'd say the majority of my work would be converting the legacy, manually-configured environments to Saltstack states and de-dockerizing some stuff. I've also been helping out with overhauling monitoring/alerting since it was previously not functioning very well.
A good overview can be seen in our monthly meeting minutes:
https://pad.documentfoundation.org/p/infra
Our git repos are private, unfortunately - more of a precaution than damning us for bad security practices. :)
On 10/25/18 4:26 PM, Brett Cornwall via aur-general wrote:
I am being sponsored by dvzrv.
I've been working in the AUR since 2014 as 'Ainola'.
Hi Brett,
some small questions and hints first:
It looks like several packages have different issues preventing to build in clean chrooted environments properly. Did you take a look at the devtools package and building packages in clean chroots so far?
What software/tool do you using to track all the new ustream releases?
You still seem to use `mksrcinfo` for generating SRCINFO files, it was deprecated in favor of native `makepkg --printsrcinfo` you may want to use that in the future.
I have noticed that mostly all git packages lack sufficient provides/conflicts on the basic non-git name schema and/or makedepends on git itself, would be nice to keep in mind
Also i notices there are multiple packages that store a tarball in the AUR source repo that contain things like icons, please don't miss-use the AUR as a storage for tarball artifacts.
some findings after reading your PKGBUILDS, can you please take a look and process the following feedback:
ags - upstream Makefile doesn't seem to respect CPPFLAGS try to either get a patch upstream or if that fails extend CFLAGS with CPPFLAGS in the PKGBUILD.
creeper-world - upstream source/url provides a https endpoint - you can't reference the PKGBUILD like in prepare(), try building in a clean chroot via extra-x86_64-build - don't think pkgdesc should ever end with a dot
creeper-world2 - upstream source/url provides a https endpoint - don't think pkgdesc should ever end with a dot - unzip to prepare the env is more of a prepare() candidate
creeper-world3 - upstream source/url provides a https endpoint - you can't reference the PKGBUILD like in prepare(), try building in a clean chroot via extra-x86_64-build - not a big fan of fiddling with PKGEXT even if its "just the AUR" but well
csound-blue - please don't use the AUR to store a tar.gz as a source - provides/conflicts on itself doesn't do anything useful - is there a reason for not stripping? - this looks like a -bin package as you are re-distributing pre-compiled files, why not just build from source instead? :)
gam - don't think pkgdesc should ever end with a dot - there is some unused client_secrets.patch sitting around in the source repo - python2-oauth2client dependency is missing - this package create cluttering files across the filesystem if you run gam as root, it will have untracked pyc files in /usr/share/gam/ you need to compile them if no distutils is provided
gnome-xcf-thumbnailer - prepare() shall never package into $pkgdir
gtk-theme-adwaita-tweaks - don't think pkgdesc should ever end with a dot
gtk-theme-adwaita-tweaks-git - lack of provides/conflicts - should pull via git+https:// - don't think pkgdesc should ever end with a dot - git makedepends is missing - the dark variant isn't published as "Adwaita Tweaks Dark" i don't think this works compared to the non git variant, it should honor the assets-dark folders and mimic what the release tarballs provide
gtk-theme-minwaita - don't think pkgdesc should ever end with a dot
hib-dlagent - none-url.patch is not a very unique name for a remote soruce, it could potentially clash, prepend with $pkgname can help
imv-git - lack of provides imv
i3lock-media-keys - don't think pkgdesc should ever end with a dot
interception-ctrl2esc-git - don't think pkgdesc should ever end with a dot - git makedepends is missing - provides/conflicts not proper - is there a reason to have interception- prefix? imo ctrl2esc-git would be the better naming here plus provides/conflicts on ctrl2esc - you should use CMAKE_INSTALL_PREFIX=/usr and DESTDIR for $pkgdir
invada-studio-plugins-lv2 - don't think pkgdesc should ever end with a dot
lazy-ips-git - don't think pkgdesc should ever end with a dot - git makedepends is missing - you can't reference the PKGBUILD like in prepare(), try building in a clean chroot via extra-x86_64-build - lack of provides/conflicts
linkchecker - don't think pkgdesc should ever end with a dot
minecraft-technic-launcher - don't think pkgdesc should ever end with a dot
nexuiz - don't think pkgdesc should ever end with a dot - upstream source/url both provides a https endpoint - not a big fan of fiddling with PKGEXT even if its "just the AUR" but well - extracting and patching should be done in prepare() - please don't use the AUR to store a tar.gz as a source, there is also an orphan bin-links.tar.gz file additionally to the used nex-icons.tar.gz checked into the AUR
pam_encfs - doesn't respect neither CFLAGS, CPPFLAGS, LDFLAGS please make sure this is always applied for c/cpp
pd-extended - non unique filename that only contains the $pkgname, must always be unique including the pkgver so avoid clashes - doesn't respect neither CFLAGS, CPPFLAGS, LDFLAGS please make sure this is always applied for c/cpp - orphan file makefile.am.patch in the tree - upstream source/url both provides a https endpoint
plasma5-applets-plasma-pass-git - don't think pkgdesc should ever end with a dot - conflicts missing on plasma5-applets-plasma-pass
python-google-auth-httplib2-git - this is a split package creating two artifacts, you should not have provides/conflicts on a global level, you need to put them in the separate variants package_*() function once for python- and once for python2- - git makedepends is missing
python-humblebundle - is there any reason to not just pull from github which also includes the LICENSE? currently the LICENSE is separately pulled from git master and it doesn't even have any unique file identifiers
python-image - don't think pkgdesc should ever end with a dot - don't use the hash based pythonhosted.org endpoint, there is a non changing variant that is more suitable for packaging as it doesn't very other then the $pkgver
python-image-git - don't think pkgdesc should ever end with a dot - git makedepends is missing
python-oath - don't think pkgdesc should ever end with a dot - each variant must at least depend in its package_* func to at least python/python2 - there are neat tests in the sources, please try to run a check() function
python-vipaccess - don't think pkgdesc should ever end with a dot - you should add all the depends from the package_* funcs to the makedepends so the check functions don't start to uselessly download packages via pip - setuptools is a hard depends and must be added in package_* as the entry_point feature is used in the setup.py for /usr/bin/vipaccess - /usr/bin/vipaccess is provided in both variants it should by something like vipaccess-py2 but this looks like a pure CLI tool that is not really usable as a library, is it? If so, it would be appropriate to just purge the python2 variant.
python2-image-git - don't think pkgdesc should ever end with a dot - git makedepends is missing - why not use a split package via python-image-git like you do for python-image? - conflicts missing on python2-image - LICENSE and README should be distributed, like in python-image-git
residualvm - don't think pkgdesc should ever end with a dot
scanbd - please don't use a install file to reference to the wiki, its not meant to be used for such general purpose documentation - upstream source provides a https endpoint
ttf-material-wifi-icons-git - should pull via git+https:// - lack of provides/conflicts
unruu - should use unique filename prefix with $pkgname-$pkgver - should just pass --prefix=/usr to ./configure and call a proper make DESTDIR="${pkgdir}" install
unruu-git - git makedepends is missing - lack of provides/conflicts - should just pass --prefix=/usr to ./configure and call a proper make DESTDIR="${pkgdir}" install
cheers, Levente
On 11/5/18 1:48 PM, Levente Polyak via aur-general wrote:
gnome-xcf-thumbnailer
- prepare() shall never package into $pkgdir
That's a write error, makepkg explicitly runs chmod on "${pkgdir}" in order to strip read/write permissions and forbid you from touching it before package() is run: https://git.archlinux.org/pacman.git/tree/scripts/makepkg.sh.in#n1679
...
Actually, come to think of it, if you try touching "${pkgdir}" during prepare() then it won't exist yet, and then after prepare() does its thing we forcibly remove the directory anyway, recreate it, and set the restrictive permissions. I wonder if it's a bug that we don't error out harder on this...
But depending on whether/how makepkg errors in the previous makepkg attempt, this directory will still exist and it will indeed be chmod a-rw at the time prepare is executed.
The rest of the time, install -d has hidden the fact that the directory simply does not exist at all.
Either way, this file is definitely not being packaged.
On 11/05/18 07:48pm, Levente Polyak via aur-general wrote:
Hi Brett,
some small questions and hints first:
Thank you for such a thorough vetting, Levente!
I'm fixing these ASAP.
On 11/05/18 07:48pm, Levente Polyak via aur-general wrote:
some small questions and hints first:
I'm nearly done with following your excellent suggestions but I have responses and questions.
It looks like several packages have different issues preventing to build in clean chrooted environments properly. Did you take a look at the devtools package and building packages in clean chroots so far?
I must sheepishly apologize. These new tools simplify everything.
What software/tool do you using to track all the new ustream releases?
urlwatch on a daily timer.
You still seem to use `mksrcinfo` for generating SRCINFO files, it was deprecated in favor of native `makepkg --printsrcinfo` you may want to use that in the future.
Thank you, switched!
I have noticed that mostly all git packages lack sufficient provides/conflicts on the basic non-git name schema and/or makedepends on git itself, would be nice to keep in mind
A silly oversight that will be enforced now that I'm learned in the ways of proper tooling.
Also i notices there are multiple packages that store a tarball in the AUR source repo that contain things like icons, please don't miss-use the AUR as a storage for tarball artifacts.
I should have known better and - at the very least - removed them before submitting my application. I've taken care of nearly all of them and have bowed my head in shame.
- don't think pkgdesc should ever end with a dot
The descriptions are often sentences, so would it not reason to end them with a period?
- not a big fan of fiddling with PKGEXT even if its "just the AUR" but
well
For a package destined for the repositories I would not fiddle and endeavor to reverse such fiddling; however, the compression time for large games is enormous only to decompress right after. Should it, for the sake of correct-ness, be reversed even for the packages doomed forever to live in the AUR?
interception-ctrl2esc-git
- is there a reason to have interception- prefix? imo ctrl2esc-git would
be the better naming here plus provides/conflicts on ctrl2esc
I normally agree (and I originally had it named that way), however...
- This is an 'interception tools' plugin... not reason enough to have the package name change, but.. - caps2esc is an older X program, so interception's variant had to be named 'interception-caps2esc'. Naming this 'interception-ctrl2esc' follows the pattern for consistency/less confusion.
With that said, should it still be named ctrl2esc?
On 07 Nov 2018, at 5:41 pm -0700, Brett Cornwall via aur-general wrote:
On 11/05/18 07:48pm, Levente Polyak via aur-general wrote:
- don't think pkgdesc should ever end with a dot
The descriptions are often sentences, so would it not reason to end them with a period?
In the case of actual sentences, it's just convention to be consistent with the packages whose descriptions are sentence fragments. It's a bit arbitrary, granted.
- not a big fan of fiddling with PKGEXT even if its "just the AUR" but
well
For a package destined for the repositories I would not fiddle and endeavor to reverse such fiddling; however, the compression time for large games is enormous only to decompress right after. Should it, for the sake of correct-ness, be reversed even for the packages doomed forever to live in the AUR?
It's just not the prerogative of the AUR contributor to make that decision for the end-user who's going to be building the package. They can very easily configure their own makepkg.conf to use uncompressed tarballs if they so desire. Basically, a PKGBUILD shouldn't override makepkg.conf unless there's a very good reason.
Cheers, Ivy
On 11/05/18 07:48pm, Levente Polyak via aur-general wrote:
creeper-world2
I've had two AUR requests queued up for some time now; deleting this package is one of them.
On Wed, Nov 07, 2018 at 05:45:01PM -0700, Brett Cornwall via aur-general wrote:
On 11/05/18 07:48pm, Levente Polyak via aur-general wrote:
creeper-world2
I've had two AUR requests queued up for some time now; deleting this package is one of them.
Quick update: I addressed your request of deleting the package. That simplifies everyone's review queue :)
Thanks, -Santiago
Hello Brett.
I took some time to randomly sample your PKGBUILDs and give some feedback:
- ags: - it appears that you use sed to change CFLAGS in the makefile definition, although it appears that the Makefile itself lets you overwrite them. I'd advice trying to use native tooling as possible, and to try to get familiar with the toolchain of each package as much as possible. - The optdepends description on wine is a bit confusing in my opinion. - I marked the package as out-of-date, as there appears to be a new version (3.1.4.15) as of almost two months ago. - I noticed that you didn't add a LICENSE file for this package.
- hib-dlagent: - I see that you backported a patch on this and ags. I was rather surprised to see that neither patches were added to new tags/releases. You could, however, cherry pick the commits rather than depending on the github api (which can change) to compute the diff for you. For this, you could use the git transport on makepkg. - I noticed that you didn't add a LICENSE file for this package.
- gam-git: - I'm not sure if this would work when built in a chroot due to those touch calls. - After reviewing the package I doubt this doesn't need a build() step. Otherwise I'd label this package a -bin. This is something that we should take special consideration of, since we could be unwittingly be introducing binaries that aren't hardened when building. (I could be wrong on this one, since it for some reason vendors many well-known packages inside of it. Good job for not pulling it those vendored deps :D) - I'm confused as to why gam.py needs to be put inside /usr/share/gam and add a .sh entrypoint for it in /usr/bin. The file seems to have a shebang and be executable... - I see that here you *also* are providing a patch. I also could find that you submitted an issue upstream for said patch (but not the patch itself)[1]. I like your initiative! Do try to keep the number of backported patches to a minimum to keep things manageable. - I noticed that you didn't add a LICENSE file for this package.
I will probably send more feedback, but I also don't want to overwhelm you with this and all the other reviews around.
Cheers! -Santiago.
On 11/7/18 9:28 PM, Santiago Torres-Arias via aur-general wrote:
Hello Brett.
I took some time to randomly sample your PKGBUILDs and give some feedback:
- ags:
- it appears that you use sed to change CFLAGS in the makefile definition, although it appears that the Makefile itself lets you overwrite them. I'd advice trying to use native tooling as possible, and to try to get familiar with the toolchain of each package as much as possible.
- The optdepends description on wine is a bit confusing in my opinion.
- I marked the package as out-of-date, as there appears to be a new version (3.1.4.15) as of almost two months ago.
- I noticed that you didn't add a LICENSE file for this package.
This checks out, Artistic2.0 is a common license.
- hib-dlagent:
- I see that you backported a patch on this and ags. I was rather surprised to see that neither patches were added to new tags/releases. You could, however, cherry pick the commits rather than depending on the github api (which can change) to compute the diff for you. For this, you could use the git transport on makepkg.
I don't see why you'd need the overhead of git for this, and that url is not going to change. They "document" it here: https://blog.github.com/2011-10-21-github-secrets/#diff-patch
They've provided it for a very long time specifically in order to let people do this, they're not going to change the scheme and render it useless for the very purpose it was created for. :p
...
cgit and gitweb let you download patch files too, GitHub just doesn't expose a visible link for it.
- I noticed that you didn't add a LICENSE file for this package.
PKGBUILD claims to be GPL2, which is a common license, but upstream has no licensing information I can find...
- gam-git:
Am I missing something? I only see a "gam" package.
- I'm not sure if this would work when built in a chroot due to those touch calls.
Are you referring to the ones in package()? Not sure why upstream code needs such weird things but AFAICT it should not break just because of a chroot.
- After reviewing the package I doubt this doesn't need a build() step. Otherwise I'd label this package a -bin. This is something that we should take special consideration of, since we could be unwittingly be introducing binaries that aren't hardened when building.> (I could be wrong on this one, since it for some reason vendors many well-known packages inside of it. Good job for not pulling it those vendored deps :D)
Looks like it just copies a couple python modules into a directory and then creates a wrapper script to run them. What would you suggest running in build(), exactly?
Devendoring looks good to me too, though.
- I'm confused as to why gam.py needs to be put inside /usr/share/gam and add a .sh entrypoint for it in /usr/bin. The file seems to have a shebang and be executable...
I'm not sure what the script does either. gam.py does do:
import utils from var import *
Which should really be explicit relative imports, but it's actually legal in python2, and there doesn't seem to be any other reason to want to cd to the directory, but the script does not cd there anyway.
- I see that here you *also* are providing a patch. I also could find that you submitted an issue upstream for said patch (but not the patch itself)[1]. I like your initiative! Do try to keep the number of backported patches to a minimum to keep things manageable.
I see two mostly similar issues, the other one has a diff referenced.
https://github.com/jay0lee/GAM/issues/created_by/ainola
Worth noting, it's much easier to get upstream projects to accept these if you provide a button they can click on to implement it -- that means pull requests.
- I noticed that you didn't add a LICENSE file for this package.
Once again, Apache license is a common license and doesn't need to be installed.
- I noticed that you didn't add a LICENSE file for this package.This checks out, Artistic2.0 is a common license.
Yes, my bad. For this and the rest of the licenses below I assumed it was the same case as MIT and such.
- hib-dlagent:
- I see that you backported a patch on this and ags. I was rather surprised to see that neither patches were added to new tags/releases. You could, however, cherry pick the commits rather than depending on the github api (which can change) to compute the diff for you. For this, you could use the git transport on makepkg.
I don't see why you'd need the overhead of git for this, and that url is not going to change. They "document" it here: https://blog.github.com/2011-10-21-github-secrets/#diff-patch
They are not even using an API stable url here. I hope it doesn't change, but it wouldn't be the first time I get biten by api's like this[1].
They've provided it for a very long time specifically in order to let people do this, they're not going to change the scheme and render it useless for the very purpose it was created for. :p
It happened to me with gitlab and their releases url, which started defaulting to "I don't recognize this branch parameter, so here's the tarball for master"[1]
- I'm not sure if this would work when built in a chroot due to those touch calls.... Are you referring to the ones in package()? Not sure why upstream code needs such weird things but AFAICT it should not break just because of a chroot.
Hmm, I see they are named as noupdatecheck and nobrowser. I assume these are to store the program state and thus need be user read-writeable? This is just speculation, hence the "I'm not sure".
- After reviewing the package I doubt this doesn't need a build() step. Otherwise I'd label this package a -bin. This is something that we should take special consideration of, since we could be unwittingly be introducing binaries that aren't hardened when building>Looks like it just copies a couple python modules into a directory and then creates a wrapper script to run them. What would you suggest running in build(), exactly?
I'm not entirely sure, I see that there's a buildscript using pyinstaller, although I'm not sure why exactly...
- I'm confused as to why gam.py needs to be put inside /usr/share/gam and add a .sh entrypoint for it in /usr/bin. The file seems to have a shebang and be executable...I'm not sure what the script does either. gam.py does do:
import utils from var import *
Which should really be explicit relative imports, but it's actually legal in python2, and there doesn't seem to be any other reason to want to cd to the directory, but the script does not cd there anyway.
On 11/7/18 10:44 PM, Santiago Torres-Arias via aur-general wrote:
It happened to me with gitlab and their releases url, which started defaulting to "I don't recognize this branch parameter, so here's the tarball for master"[1]
Yes, gitlab is prone to bugs like this. :p
gitlab also includes the version of git(1) inside the patch file, and moved the tarball endpoint for legitimately good reasons in https://gitlab.com/gitlab-org/gitlab-ce/issues/38830
Because their tarballs used to be insane. And giving you the tarball for master is a regression they fixed, not an API they dropped. :p
- I'm not sure if this would work when built in a chroot due to those touch calls.... Are you referring to the ones in package()? Not sure why upstream code needs such weird things but AFAICT it should not break just because of a chroot.
Hmm, I see they are named as noupdatecheck and nobrowser. I assume these are to store the program state and thus need be user read-writeable? This is just speculation, hence the "I'm not sure".
It should be owned by root unless some process uses something like install -o username -g groupname.
Looks like it just copies a couple python modules into a directory and then creates a wrapper script to run them. What would you suggest running in build(), exactly?
I'm not entirely sure, I see that there's a buildscript using pyinstaller, although I'm not sure why exactly...
Most likely in order to create some giant windows executable that ships the entire python application runtime, plus the gam source code, in order to spare Windows users the need to install Python.
On Wed, Nov 07, 2018 at 11:04:43PM -0500, Eli Schwartz via aur-general wrote:
Because their tarballs used to be insane. And giving you the tarball for master is a regression they fixed, not an API they dropped. :p
It's an API url they dropped. They moved it around and instead of 404'd they just left it broken.
True though, probably github is not prone to things like this...
It should be owned by root unless some process uses something like install -o username -g groupname.
Ah, I'm still not sure if the executable should setuid to update than when run as a user (although it shouldn't be able if it's just a script).
Looks like it just copies a couple python modules into a directory and then creates a wrapper script to run them. What would you suggest running in build(), exactly?
I'm not entirely sure, I see that there's a buildscript using pyinstaller, although I'm not sure why exactly...
Most likely in order to create some giant windows executable that ships the entire python application runtime, plus the gam source code, in order to spare Windows users the need to install Python.
I think you're right, but I'm still confused as to why there's a linux vaiant of it...
https://github.com/jay0lee/GAM/blob/master/src/linux-build.sh
Probably for the same reason as you pointed out though...
-Santiago
On 11/07/18 09:28pm, Santiago Torres-Arias wrote:
Hello Brett.
I took some time to randomly sample your PKGBUILDs and give some feedback:
- ags:
- it appears that you use sed to change CFLAGS in the makefile definition, although it appears that the Makefile itself lets you overwrite them. I'd advice trying to use native tooling as possible, and to try to get familiar with the toolchain of each package as much as possible.
I will admit that I didn't think to go through those lines when I inherited it. You're totally right, there's no reason to do it that way.
- The optdepends description on wine is a bit confusing in my opinion.
I removed it. There's little reason to have it there anyway. The optdepends isn't a good place to inform people about that anyway.
- I marked the package as out-of-date, as there appears to be a new version (3.1.4.15) as of almost two months ago.
Long story short, that was pretty much exactly during the time when I accidentally clobbered my urlwatch file. Thanks for bringing that up to me.
- I noticed that you didn't add a LICENSE file for this package.
Artistic2.0 is a uncommonly used common license! (/usr/share/licenses/common/Artistic2.0/license.txt)
- hib-dlagent:
- I see that you backported a patch on this and ags. I was rather surprised to see that neither patches were added to new tags/releases. You could, however, cherry pick the commits rather than depending on the github api (which can change) to compute the diff for you. For this, you could use the git transport on makepkg.
That would bring another dependency on git, though. I can surely do if if it's more 'correct' but I wouldn't imagine that Github would change that API anytime soon.
Or would it be better to just carry the patch locally in the repo?
- I noticed that you didn't add a LICENSE file for this package.
Yikes, the project doesn't even have a license! I should have checked that when I inherited it (the packager just slapped a GPL2 on it). Really, I had just uploaded it so it wouldn't have been lost after the AUR 4 migration.
I'll bug upstream about it.
- gam-git:
- I'm not sure if this would work when built in a chroot due to those touch calls.
- After reviewing the package I doubt this doesn't need a build() step. Otherwise I'd label this package a -bin. This is something that we should take special consideration of, since we could be unwittingly be introducing binaries that aren't hardened when building. (I could be wrong on this one, since it for some reason vendors many well-known packages inside of it. Good job for not pulling it those vendored deps :D)
- I'm confused as to why gam.py needs to be put inside /usr/share/gam and add a .sh entrypoint for it in /usr/bin. The file seems to have a shebang and be executable...
- I see that here you *also* are providing a patch. I also could find that you submitted an issue upstream for said patch (but not the patch itself)[1]. I like your initiative! Do try to keep the number of backported patches to a minimum to keep things manageable.
- I noticed that you didn't add a LICENSE file for this package.
Of all the packages you had to click on that one. :(
I know it doesn't really excuse it but gam is sort of a "WIP" because it's... oddly written. I've been meaning to set aside some time to get some patches in to make it more palatable for packaging. The patch is a complete hack right now just to make the package "work" (when I inherited it it was FUBAR).
I will probably send more feedback, but I also don't want to overwhelm you with this and all the other reviews around.
I really appreciate the feedback! It always sucks when so many little things become so glaring after the fact but
- I marked the package as out-of-date, as there appears to be a new version (3.1.4.15) as of almost two months ago.
Long story short, that was pretty much exactly during the time when I accidentally clobbered my urlwatch file. Thanks for bringing that up to me.
- I noticed that you didn't add a LICENSE file for this package.
Artistic2.0 is a uncommonly used common license! (/usr/share/licenses/common/Artistic2.0/license.txt)
Yes, my bad. I was told about this on MIT, and I assumed this was the case for most licenses...
- hib-dlagent:
- I see that you backported a patch on this and ags. I was rather surprised to see that neither patches were added to new tags/releases. You could, however, cherry pick the commits rather than depending on the github api (which can change) to compute the diff for you. For this, you could use the git transport on makepkg.
That would bring another dependency on git, though. I can surely do if if it's more 'correct' but I wouldn't imagine that Github would change that API anytime soon.
Or would it be better to just carry the patch locally in the repo?
True, I didn't consider the dependency on git. I'd say you could check it in. I do not agree with Eli that you should rely on api's like this to get a simple patch. It has been my experience that api's like this move around and leave you trying to debug weird errors.
- I noticed that you didn't add a LICENSE file for this package.
Yikes, the project doesn't even have a license! I should have checked that when I inherited it (the packager just slapped a GPL2 on it). Really, I had just uploaded it so it wouldn't have been lost after the AUR 4 migration.
I'll bug upstream about it.
- gam-git:
...
Of all the packages you had to click on that one. :(
I know it doesn't really excuse it but gam is sort of a "WIP" because it's... oddly written. I've been meaning to set aside some time to get some patches in to make it more palatable for packaging. The patch is a complete hack right now just to make the package "work" (when I inherited it it was FUBAR).
Yes, granted I'm rather confused when I read the repository and see things like build-linux.sh that pulls pyinstaller. I didn't know exactly what of all was happening there...
I will probably send more feedback, but I also don't want to overwhelm you with this and all the other reviews around.
I really appreciate the feedback! It always sucks when so many little things become so glaring after the fact but
Lol I've been there, no worries :)
-Santiago.
Le 08/11/2018 à 04:34, Santiago Torres-Arias via aur-general a écrit :
- I noticed that you didn't add a LICENSE file for this package.
Artistic2.0 is a uncommonly used common license! (/usr/share/licenses/common/Artistic2.0/license.txt)
Yes, my bad. I was told about this on MIT, and I assumed this was the case for most licenses...
We have a instructions here: https://wiki.archlinux.org/index.php/PKGBUILD#license (which redirects to the actual licenses package for a list of what is common). ;)
- hib-dlagent:
- I see that you backported a patch on this and ags. I was rather surprised to see that neither patches were added to new tags/releases. You could, however, cherry pick the commits rather than depending on the github api (which can change) to compute the diff for you. For this, you could use the git transport on makepkg.
That would bring another dependency on git, though. I can surely do if if it's more 'correct' but I wouldn't imagine that Github would change that API anytime soon.
Or would it be better to just carry the patch locally in the repo?
True, I didn't consider the dependency on git. I'd say you could check it in. I do not agree with Eli that you should rely on api's like this to get a simple patch. It has been my experience that api's like this move around and leave you trying to debug weird errors.
Please don’t start cloning a repo just for some small patches that can be retrieved by this stable and long-lived GitHub API. And @Brett, no, you should not carry the patch locally. No reason to clobber our tree with that. ;)
Regards, Bruno
As the (newish, but still not updated [1]) discussion period of 14 days is now over, I have started the vote.
Thanks again to all reviewers and active TUs in this discussion period! I think Brett will make a good TU!
Let's vote! :)
[1] https://aur.archlinux.org/trusted-user/TUbylaws.html#_standard_voting_proced...
On 11/9/18 1:05 PM, David Runge wrote:
As the (newish, but still not updated [1]) discussion period of 14 days is now over, I have started the vote.
Thanks again to all reviewers and active TUs in this discussion period! I think Brett will make a good TU!
Let's vote! :)
[1] https://aur.archlinux.org/trusted-user/TUbylaws.html#_standard_voting_proced...
I got worried for a second there, but the section you're looking at is the default "procedure for general proposals" when another section doesn't override the five-day discussion period.
This definitely lists the right time period: https://aur.archlinux.org/trusted-user/TUbylaws.html#_addition_of_a_tu
On 2018-11-09 13:11:19 (-0500), Eli Schwartz via aur-general wrote:
This definitely lists the right time period: https://aur.archlinux.org/trusted-user/TUbylaws.html#_addition_of_a_tu
Yep, sorry, that was just a brainfart from my side. ;-)
Here's the link to the vote btw: https://aur.archlinux.org/tu/?id=112
On 2018-11-09 19:05:58 (+0100), David Runge wrote:
Let's vote! :)
The results are in (the vote ended nearly an hour ago)!
Yes: 26 No: 9 Abstain: 12 Participation 90.38% (come on, don't drop the ball on this! ;-))
I have just updated your account status in the AUR to 'Trusted User'. Congratulations!
Please follow up on the TODO for new Trusted Users [1].
See you on IRC :)
Bests, David
[1] https://wiki.archlinux.org/index.php/AUR_Trusted_User_Guidelines#TODO_list_f...
On 11/16/18 08:11pm, David Runge wrote:
The results are in (the vote ended nearly an hour ago)!
Thank you all.
To those that voted 'No': Feel free to send me a message with your feedback. Your concerns would be a valuable asset for me to keep in mind.
aur-general@lists.archlinux.org