Code review comment for lp:~nigelbabu/launchpad/bug-title-849121

Hi Nigel, thanks a lot for this branch. Thanks for fixing the lint errors. It looks great but I spotted a few things I'd like you too fix before we land this.

[1]

30 - bug_ids = getUtility(IBugTaskSet).searchBugIds(params)
31 - invalid = set(bugs) - set(bug_ids)
32 - for bug in invalid:
33 + bugtasks = getUtility(IBugTaskSet).search(params)
34 + for task in bugtasks:
35 + valid_links['/bugs/' + str(task.bug.id)] = task.bug.title
36 + bugs.remove(task.bug.id)
37 + for bug in bugs:

Although the logic seems fine, I think this would be more clear if you iterate over a variable named 'invalid_bugs' (or something) like this was done before instead of creating that list dynamically like you do now.

[2]

137 + } else if( (href in valid_links) ) {

I think you can remove some parenthesis here.

9 - // ATM, we only handle +branch links.
150 + // ATM, we only handle +branch and bug links

Nitpick: please add the '.' at the end of the sentence ;).

[3]

257 + // Test the setup method.

I think this is a left-over ...?

[4]

171 + if( console !== undefined ) {

I know this is not your code in the first place but since you're fixing it (and doing a good job at it), you could use YUI's Y.Lang.isValue (see http://developer.yahoo.com/yui/docs/YAHOO.lang.html) to check if console is undefined.

[5]

184 + process_links(link_info, 'branch-short-link',
185 'branch_links');
186 - process_invalid_links(link_info, 'bug-link',
187 + process_links(link_info, 'bug-link',
188 'bug_links');

Again, not your code but maybe you could fix the indentation here.

[6]

261 + links.check_valid_lp_links(mock_io);
262 + var response_json = ['{"bug_links": {"valid": {"/bugs/14"',
263 + ': "jokosher exposes personal details in its actions portlet"}',
264 + ', "invalid": {"/bugs/200": "Bug 200 cannot be found"}}, ',
265 + '"branch_links": {"invalid": {"/+branch/invalid": "No such',
266 + ' product: \'foobar\'."}}}'].join('');

My eyes! Formatting it this way makes it very difficult to read. Because you're using mockio you must build a string but my advice here would be to create the json object (properly formatted) and apply Y.JSON.stringify to it.

[7]

278 + Y.Assert.areSame(
279 + 'jokosher exposes personal details in its actions portlet',
280 + validbug.get('title'));
281 + },
282 + test_branch: function () {

2 small formatting problem here:
- please add an empty line between the functions;
- please fix the indentation of the line which starts with "'jokosher" and the one that follow.