Review: Approve code*

Thanks Ian, couple of comments:

- I'd suggest creating some methods to use for the Array.each bits that make 
for some code that'a pretty complex to figure out what it's doing. This is for 
lines 108 of the diff. The test is a bit unclear as well in line 187. 

It might even be a case where just nested if's are more clear with the line 
breaks than the single big if condition.

- The title of the control is set to 'There are no shared artifacts.' which 
will be shown to the user on hover in most browsers. Is artifacts the right 
term here for general users? Is there something more clear that they're used to 
seeing? 
-- 
https://code.launchpad.net/~wallyworld/launchpad/update-sharing-policies-some-988630/+merge/103626
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to