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
signature.asc
Description: PGP signature
