[aur-dev] [PATCH] Add explanation for Popularity field in search results

Lukas Fleischer lfleischer at archlinux.org
Mon Jun 15 07:59:00 UTC 2015


Thanks, looks better than the initial submission! Some comments below.

On Sun, 14 Jun 2015 at 23:29:34, Leonidas Spyropoulos wrote:
> Fixes FS#45327
> 

We usually end sentences (and half sentences) with a full stop in the
commit message body. I know this seems really nit-picky but I thought
I'd mention it, since you will probably revise the patch once more
anyway.

> Signed-off-by: Leonidas Spyropoulos <artafinde at gmail.com>
> ---
>  web/html/css/aurweb.css             | 6 ++++++
>  web/template/pkg_search_results.php | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/web/html/css/aurweb.css b/web/html/css/aurweb.css
> index a3f43bd..ba98914 100644
> --- a/web/html/css/aurweb.css
> +++ b/web/html/css/aurweb.css
> @@ -85,3 +85,9 @@ legend {
>  p.important {
>         font-weight: bold;
>  }

Missing newline to separate the blocks.

> +span.hover-help {
> +       border-bottom: 1px dotted black;
> +       background-color:#FFFFFF;
> +       color:#000000;
> +       text-decoration:none;

Do we really need to specify background-color, color and text-decoration
here? Shouldn't

    span.hover-help {
    	border-bottom: 1px dotted black;
    }

do what we want?

> +}
> diff --git a/web/template/pkg_search_results.php b/web/template/pkg_search_results.php
> index 13d9bfc..f06ba18 100644
> --- a/web/template/pkg_search_results.php
> +++ b/web/template/pkg_search_results.php
> @@ -35,7 +35,7 @@ if (!$result): ?>
>                                         <th><a href="?<?= mkurl('SB=n&SO=' . $SO_next) ?>"><?= __("Name") ?></a></th>
>                                         <th><?= __("Version") ?></th>
>                                         <th><a href="?<?= mkurl('SB=v&SO=' . $SO_next) ?>"><?= __("Votes") ?></a></th>
> -                                       <th><a href="?<?= mkurl('SB=p&SO=' . $SO_next) ?>"><?= __("Popularity") ?></a></th>
> +                                       <th><a href="?<?= mkurl('SB=p&SO=' . $SO_next) ?>"><?= __("Popularity") ?></a><span title="Popularity is calculated as the sum of all votes with each vote being weighted with a factor of 0.98 per day since its creation." class="hover-help"><sup>?</sup></span></th>

We want the explanation to be translated, so the __() keyword is
required here:

    [...]<span title="<?= __('Popularity is [...] creation.') ?>">[...]

Thanks!

>                                         <?php if ($SID): ?>
>                                         <th><a href="?<?= mkurl('SB=w&SO=' . $SO_next) ?>"><?= __("Voted") ?></a></th>
>                                         <th><a href="?<?= mkurl('SB=o&SO=' . $SO_next) ?>"><?= __("Notify") ?></a></th>
> -- 
> 2.4.3


More information about the aur-dev mailing list