[Koha-patches] [PATCH 50/78] acqui-home new links to budget and dealing with budget instead of bookfund

Joe Atzberger joe.atzberger at liblime.com
Tue Jun 2 23:25:37 CEST 2009


This tag <br /> is valid xhtml, but <br> is not.  The latter appears at
least 9 times in this patch.

Using the built-in loop context variable for __odd__ is preferable to the
old-style toggle code like:

+   <!--TMPL_IF name="toggle"-->
> +      <tr class="highlight">
> +      <!--TMPL_ELSE-->
> +      <tr>
> +      <!--/TMPL_IF-->


Because the loop_context_var is exclusively in the presentation layer and
doesn't require fiddling in the script.

Using jquery $("#tableid tr:even") might be preferable to both other methods
for the staff interface.


This section also seems to result in invalid XHTML:

        <tfoot>
>         <tr>
>             <th>Total</th>
> -            <th><!-- TMPL_VAR name="total" --></th>
> -            <th><!-- TMPL_VAR name="totspent" --></th>
> -            <th><!-- TMPL_VAR name="totcomtd" --></th>
> -            <th><!-- TMPL_VAR name="totavail" --></th>
> +                <td>
> +                <td>
> +            <th><p  align="right" ><!-- TMPL_VAR name="total" --></p>
> </th>
> +            <th><p  align="right" ><!-- TMPL_VAR name="totspent" --></p>
> </th>
> +            <th><p  align="right" ><!-- TMPL_VAR name="totavail" --></p>
> </th>
>         </tr>
>         </tfoot>
>     </table>
>     </div>
>

Paul, I realize that a lot of these patches go back and fix problems
introduced by the older ones.  In such cases, it would be great if you (and
everybody else!) could use something like:
*git rebase --interactive HEAD~10
*

to "squash" related commits such that you only have to submit one combined
and corrected patch.  (The 10 is an arbitrary number of commits back that
you want to examine.)  That keeps me as a reviewer from having to reread all
28 of the subsequent patches to see if you already caught these problems,
and it prevents coders and the RM from being able to break things by only
applying only up to the broken patch.

In general, squashing is a great way to keep all the pieces of a feature
together and prevent the situation where a single one-line patch being left
out will break everything else.  Yet another awesome git feature...

-- 
Joe Atzberger
LibLime - Open Source Library Solutions
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/koha-patches/attachments/20090602/cac7fd34/attachment-0002.htm>


More information about the Koha-patches mailing list