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

[Colin Clark]

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.

http://issues.fluidproject.org/browse/FLUID-3793

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.

Thanks,

Colin


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
http://fluidproject.org