https://bugzilla.wikimedia.org/show_bug.cgi?id=34411

Jeroen De Dauw <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from Jeroen De Dauw <[email protected]> 2012-02-23 
13:21:42 UTC ---
Great. Applied in r112201, and tested with trunk :)

Some comments though:
* Please provide patches against trunk. If someone makes a change on SVN and
you just incorporate it in your patch, merging will result in a conflict since
the code is already there.
* You where doing some false selection in the JS :) 
* If you have a dictionary of stuff in JS (ie { stuff: ... }), then don't put a
comma after the last element, this makes IE cry (yeah, really >_>)
* Avoid aligning assignments in most cases, can be useful in setup files with
long lists of similar assignments, but rarely in other places.

One improvement possible in the JS is getting rid of all the var and just
assigning directly in the dictionary, like this:

( '#carousel' ).jcarousel( {
    parseInt($( '#carousel' ).attr( 'scroll' ) ),
    ...

-- 
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to