launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

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.