Hi Rick Thanks for the review.
> > - 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. > I'll see if I can make it clearer. It was much easier to read before I had to artificially chop up the lines due to our ridiculously small line lengths. I mean, come on, 80 chars in 2012? It's no longer 1972. Sorry, ranting, this is a bugbear of mine. > - 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? I'll change to "bugs or branches". We can also get feedback on the wording and change if required. We're still in beta so we have a little latitude. -- 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
