Thanks for the new review! I have sent a v2 PATCH, comments inline below.
On 29-01-18, Lukas Fleischer wrote:
> > diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
> > index 9eeaafd..30bdc89 100644
> > --- a/web/lib/aurjson.class.php
> > +++ b/web/lib/aurjson.class.php
> > @@ -17,7 +17,8 @@ class AurJSON {
> > 'suggest-pkgbase', 'get-comment-form'
> > );
> > private static $exposed_fields = array(
> > - 'name', 'name-desc', 'maintainer'
> > + 'name', 'name-desc', 'maintainer',
> > + 'depends', 'makedepends', 'checkdepends', 'optdepends'
>
> Just another idea: maybe we can put the new fields in a separate array
> $exposed_depfields and merge that array into $exposed_fields upon
> initialization. Then we would not need to hardcode them again in the
> in_array() check below.
I have moved the new fields to a variable $exposed_depfields, it indeed
makes things slightly easier to read. However, I think that merging the
two arrays later in the code would be rather confusing for the reader
(because at first glance, the dependency fields would seem to be absent
from $exposed_fields).
> This is a bit unfortunate, indeed. Can we, instead of doing this, use a
> subselect? Something like the following statement might work:
>
> $keyword_string IN (SELECT PackageDepends.DepName FROM PackageDepends
> LEFT JOIN DependencyTypes
> ON PackageDepends.DepTypeID = DependencyTypes.ID
> WHERE PackageDepends.PackageID = Packages.ID
> AND DependencyTypes.Name = $search_string)
Excellent idea, and it even worked on the first try! It simplifies the
code a lot, and performance-wise it should be roughly similar to a JOIN,
possibly a bit slower (from what I could read on various stack overflow
posts).
Baptiste
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.archlinux.org/pipermail/aur-dev/attachments/20180131/1ec96cee/attachment.asc>