launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

Review: Needs Information

#39: should be _information_type, not _inforation_type

#66: Is it necessary to prevent invalid characters from being typed? That seems like bad UI to me--if I (not knowing good urls) put in bad characters, but nothing happens, I feel like the app or my keyboard is broken. Better maybe to have a dynamic warning that comes up saying it's invalid and what the invalid character is? Or throw away the dynamics and just up the warning after typing ends/submission occurs and validation fails?

#266: Why are we writing our own render method? This will break renderUI and syncUI calls down the road if anyone wants to add that, right? Wouldn't all of this render code be better set in renderUI and called by the render cycle?

#291: Given they all say the same thing (barring the one one line #330) I'm not sure these comments help much? They're not telling me anything that I can't see just from looking--I tend to err on no comment are better than unneeded ones. If you have these b/c of auto-docs of some sort, I would prefer "Lazy load the search results node" etc over "the found node" since if I'm scanning auto-docs I'm probably not pulling up the function.


-- 
https://code.launchpad.net/~rharding/launchpad/productjs/+merge/123803
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.