This comment should be changed. Will do in next checkin.

Nit: indent 4 spaces per level.

Nit - title is indented too far. Should be:

var modal = {
  title: "Review Bot"
};

Perhaps it'd be better to inject the dialog content inside a display:none; node within the page body?  Then just manipulate the tool list on load?

.html($("<div/>").attr("id", "dialogContent")
                 .attr("class", "modalbox-contents"))

Isn't it too soon to trigger ready if the tools aren't loaded yet?

$("#dialogContent").html("Installed tools:")
                   .append(toolList);

The indentation is a bit wonky here - lines 52 to 58 look like they're indented too far.

Same wonky indentation as above.

Maybe we could style this in a way which will allow expansion. Eventually I'd like us to add information about which tools have already been executed, or have been trigger to execute. Maybe a table/grid would be a better style?

I'm not really a fan of this here, can we make it not happen.

We might want to include version information here. It's possible to have multiple versions of the same tool installed on separate workers. If that happens, we couldn't differentiate here.

I feel like we might want to namespace this a bit more. It's feasible something else could make elements with id='checkbox_<#>'.

Something more accurate might be: 'No tools available to run manually.'

I know this was just a rudimentary resource to get things working, so you can make the front-end, but I want to put some notes for the future here. Feel free to ignore these for now, and focus on the front-end work:

- This should be combined with the resource above (ReviewBotToolResource).

- Right now this pulls all tools, we should add arguments to the resource which allow you to pull only tools you want (ones which can run manually).

- Eventually we're going to be restricting which tools users can execute manually based on permissions. It might be best to only display tools which the user has permissions for.

Put an empty line between these.

We should change this to something like 'reviewbot_tool_id'

This should be moved into the initialization of the extension.

Also, when the extension is "shutdown" we should call 'unregister_resource_for_model(ReviewBotTool)'

Why the newline at the end of this? Why not:

register_resource_for_model(ReviewBotTool,
                            review_bot_installed_tool_resource)

?

Same below with the TemplateHook.

Nit: alignment

Space after if, before {

What's the first ( for?

Nit - space on either side of +

Alphabetical here

Alphabetical here

Indentation here should be:

register_resource_for_model(ReviewBotTool,
                            review_bot_installed_tool_resource)

Same comment as above

I'm not sure: should this also unregister 'review_bot_installed_tool_resource'?

I think RB js styles strives to use just 1 `var`, and separates the variables by comma.

To mentors:

Is there a difference between using:

{% get_media_prefix %}

vs.

{{MEDIA_URL}}

?

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    extension/reviewbotext/resources.py
    extension/reviewbotext/extension.py
  Ignored Files:
    extension/reviewbotext/htdocs/css/reviewbot.css
    extension/reviewbotext/templates/reviewbot_hook_action.html
    extension/reviewbotext/htdocs/js/reviewbot.js

Col: 18
 E203 whitespace before '

Col: 80
 E501 line too long (80 > 79 characters)

Col: 1
 E302 expected 2 blank lines, found 1

Col: 1
 W293 blank line contains whitespace

Col: 1
 W293 blank line contains whitespace

Col: 70
 W292 no newline at end of file

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    extension/reviewbotext/resources.py
    extension/reviewbotext/extension.py
  Ignored Files:
    extension/reviewbotext/htdocs/css/reviewbot.css
    extension/reviewbotext/templates/reviewbot_hook_action.html
    extension/reviewbotext/htdocs/js/reviewbot.js

Col: 80
 E501 line too long (80 > 79 characters)

Col: 1
 W293 blank line contains whitespace

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    extension/reviewbotext/resources.py
    extension/reviewbotext/extension.py
  Ignored Files:
    extension/reviewbotext/htdocs/css/reviewbot.css
    extension/reviewbotext/templates/reviewbot_hook_action.html
    extension/reviewbotext/htdocs/js/reviewbot.js

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    extension/reviewbotext/resources.py
    extension/reviewbotext/extension.py
  Ignored Files:
    extension/reviewbotext/htdocs/css/reviewbot.css
    extension/reviewbotext/templates/reviewbot_hook_action.html
    extension/reviewbotext/htdocs/js/reviewbot.js

Col: 51
 E226 missing optional whitespace around operator

We probably don't want to pass here. Will we assume that not passing in any selected_tools argument means that we are in the automatic trigger case? Probably should also push error msg to the user if there is a problem running the selected tool(s).