[aur-dev] Adding the HE language.

Loui Chang louipc.ist at gmail.com
Sun Aug 15 23:47:41 EDT 2010


On Mon 16 Aug 2010 01:53 +0200, PyroPeter wrote:
> Attached is a patch that adds Right-To-Left-support.
> 
> From the commit message:
> This replaces most parts of pkg_search_results.php, as this file was the
> first one I looked at more closely that had great flaws:
> * The obscure page-number-generation-code was rewritten and moved to
>   pkgfuncts.inc
>   * There are no more "Next" and "Previous" buttons, imo they are
>     redundant.
>   * The links now cover the whole page range, not just the pages next
>     to the current one.
> * The blind-table in the footer was replaced by floating divs to allow
>   proper RTL-support.
> * The odd rows of the results-table are now made darker with means of
>   CSS 3. Every arch-user's browser should support this.
> * The page is now valid HTML even if there are no results.
> * Removed all <td><span><span>content</span></span></td>-like oddities.
>   <td> is an inline element by itself, there is no need to place a
>   <span> in there. Two of them make even less sense.
> * Removed reoccurring style-attributes. Adopted CSS to do the same.
> * Changed indention to something (imo) senseful.
>   Indention is now done like in HTML; If there is an 'if' or 'for'
>   in the PHP-code, the affected HTML-elements are also indented.
> 
> If someone considers my indention scheme to be senseful, I would
> add an explaination to HACKING.

I appreciate your efforts PyroPeter. I have some suggestions though.

I recommend you to keep your patches small and focused.

Here's a guideline:
Each of those bullet points above should be at least one commit.

This huge all-in-one patch makes it very hard for me to review, and
frankly I don't like to deal with them at all.

Try to send debatable patches last - for example I'm not so sure that
removing 'next' and 'previous' is a good idea. So that applies when the
patch is a matter of opinion.

Thanks



More information about the aur-dev mailing list