invalid_links is not really an appropriate variable name now, because
the value takes the form of {"valid": ..., "invalid": ...}. Something
like link_info would be better I think.
[2]
+ # Remove valid bugs from the list of all the bugs.
+ bugs.remove(task.bug.id)
Can you change bugs to be a set rather than a list? Also, it should
probably be called bug_ids rather than bugs.
[3]
+ for task in bugtasks:
+ valid_links['/bugs/' + str(task.bug.id)] = task.bug.title
It's might just be me, but % formatting would be clearer here:
In addition to Raphaël's tip top review, I'd like to add a few points:
[1]
- result['invalid_' + link_type] = invalid_links
+ result[link_type] = invalid_links
invalid_links is not really an appropriate variable name now, because
the value takes the form of {"valid": ..., "invalid": ...}. Something
like link_info would be better I think.
[2]
+ # Remove valid bugs from the list of all the bugs. task.bug. id)
+ bugs.remove(
Can you change bugs to be a set rather than a list? Also, it should
probably be called bug_ids rather than bugs.
[3]
+ for task in bugtasks: '/bugs/ ' + str(task.bug.id)] = task.bug.title
+ valid_links[
It's might just be me, but % formatting would be clearer here:
[4]
+ if( Y.Object. size(invalid_ links) === 0 ) { size(valid_ links) === 0 ) {
+ invalid_links = {};
+ }
+ if( Y.Object.
+ valid_links = {};
+ }
I didn't know about Object.size(), neat.
It's a nitpick, but if invalid_links is {} already, this will set it
to a new {}. It just doesn't seem very neat ;) How about:
if (invalid_links === undefined) {
invalid_ links = {};
valid_ links = {};
}
if (valid_links === undefined) {
}
Or use Y.Lang.isValue().
In fact, it's common practice to use implicit truth with JavaScript,
so you can probably combine the above with initialization:
var invalid_links = link_info[ link_type] .invalid || {}; link_type] .valid || {};
var valid_links = link_info[
[5]
+ if(href in invalid_links) {
...
+ } else if(href in valid_links) {
There were lint complaints about these. For the sake of noise, change
these to use hasOwnProperty().