← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~jcsackett/launchpad/privacy-banner-filebug into lp:launchpad

 

Review: Approve

Thanks for the UX fix.

#23
Officially any time we compare against null like that standards say to use Y.Lang.isNull(). In the case of a selector, I'd just use if (itypes_table).

#48
Is there anything that might make this a bit clearer? The update_privacy_banner method's first argument is called 'show' but we're passing an variable 'private_type' which I was thinking was some sort of selection from a list, not a true/false. How about s/private_type/is_private ?


-- 
https://code.launchpad.net/~jcsackett/launchpad/privacy-banner-filebug/+merge/120474
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References