Bug Parade code review request: FLUID-3793 (a Pager bug)

Colin Clark colinbdclark at gmail.com
Tue Oct 26 15:08:47 UTC 2010

Hi Antranig,

I'm wondering if you might have a chance to review Anastasia's small change to the Pager to fix an issue that causes the user to see errors in the Pager's default configuration.


The bug, I think, stems from improper usage of a private Pager function called getRoots(). The function takes an argument and modifies it in place,  returning no value. Most places where it is used, the calling code expects these semantics (modify in place, no return). In one case--the buggy code Anastasia found--the usage is "incorrect," assuming the standard semantics one might expect for a function starting with "get" (returns a value).

Anastasia's patch normalizes the usage, but the other question it raises is if the getRoots() function is perhaps poorly named. A minor issue all around, but since you know the code base the best, we'd appreciate your feedback.



On 2010-10-12, at 2:45 PM, Cheetham, Anastasia wrote:

> I've attached a patch to
>   http://issues.fluidproject.org/browse/FLUID-3793
> to fix a bug in the Pager's selfRender body renderer. It was a small bug, but it caused our demo to fail, and would cause a failure in *any* application using the selfRender bodyRenderer with the default columnDefs configuration ("explode").
> The patch includes a test that catches the failure and passes with the fix in place.
> I'd appreciate a review, and any suggestions for improvements, before I commit the fix.
> -- 
> Anastasia Cheetham     Inclusive Design Research Centre
> acheetham at ocad.ca            Inclusive Design Institute
>                                        OCAD University
> _______________________________________________________
> fluid-work mailing list - fluid-work at fluidproject.org
> To unsubscribe, change settings or access archives,
> see http://fluidproject.org/mailman/listinfo/fluid-work

Colin Clark
Technical Lead, Fluid Project

More information about the fluid-work mailing list