Docs for URL,Dashboard,NavigationBar-Hooks

Review Request #3617 — Created Dec. 3, 2012 and submitted

Information

Review Board
master

Reviewers

Extensions documentation for URLHook, DashboardHook, 
and NavigationBarHook.
Ran `make html` in reviewboard/docs/codebase/

No error output from sphinx.

Rendered HTML pass visual inspection.
Description From Last Updated

These are only really useful if we're actively linking to them.

chipx86chipx86

"Review Board"

chipx86chipx86

Hooks aren't really there to help "customize" Extensions - they're there to give extensions a means to customize Review Board. …

mike_conleymike_conley

"Review Board"

chipx86chipx86

"We briefly examine a few...demonstrage usage with sample code" - on tone, sounds a bit text-bookish. Maybe instead, simply: "The …

mike_conleymike_conley

We can make this even more general - "URLHooks are used to extend the URL patterns that Review Board will …

mike_conleymike_conley

This last sentence is generally true for all hooks - maybe you won't need it if you add that sentence …

mike_conleymike_conley

"once is" -> "once"

chipx86chipx86

SampleExtension ? Probably don't need to capitalize "Dashboard." Is this example meant to tie in with the DashboardHook? If so, …

chipx86chipx86

Alignment error.

chipx86chipx86

"dashboard page" ?

chipx86chipx86

"on the dashboard's navigation pane."

chipx86chipx86

"dashboard page."

chipx86chipx86

"extension" "dashboard page"

chipx86chipx86

I think it's better to make this one statement.

chipx86chipx86

Blank line above.

chipx86chipx86

Make sure the indentation inside the template tags are correct: {% block content %} {% box "reports" %}

chipx86chipx86

Instead of "To do..." how about something like, "This is my new Dashboard page for Review Board!" That way, it …

mike_conleymike_conley

Probably want a blank line between these.

chipx86chipx86

End with a period.

chipx86chipx86

It'd be nice to just pass the entries in here directly.

chipx86chipx86

They don't use them as a primary mechanism, they *are* a primary mechanism. I'd be OK with "Extensions can use …

mike_conleymike_conley

Let's say this: Notice how sample_extension.urls was included in the patterns. In this case, sample_extension is the package name for …

mike_conleymike_conley

"it then" -> "then it"

mike_conleymike_conley

This looks like leftover cruft to be removed.

mike_conleymike_conley
chipx86
  1. 
      
  2. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    These are only really useful if we're actively linking to them.
    1. Roger; removed the ones not being referenced by a link somewhere.
  3. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "Review Board"
  4. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "Review Board"
  5. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "once is" -> "once"
  6. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    SampleExtension ?
    
    Probably don't need to capitalize "Dashboard." Is this example meant to tie in with the DashboardHook? If so, maybe make that clear and link to the section on that.
    
    "a URL"
  7. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
     
    Show all issues
    Alignment error.
    1. Should it be:
      
      urlpatterns = patterns('sample_extension.views',
          url(r'^$', 'dashboard'),
      )
      
      ?
      
      I grepped reviewboard repo for "urlpatterns" and they all seem to follow that structure, but have more entries for "url"
  8. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "dashboard page" ?
  9. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "on the dashboard's navigation pane."
  10. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "dashboard page."
  11. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    "extension"
    
    "dashboard page"
  12. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
     
     
     
    Show all issues
    I think it's better to make this one statement.
  13. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    Blank line above.
  14. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
     
     
     
     
     
    Show all issues
    Make sure the indentation inside the template tags are correct:
    
    {% block content %}
    {%  box "reports" %}
  15. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
    Show all issues
    Probably want a blank line between these.
  16. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    End with a period.
  17. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    It'd be nice to just pass the entries in here directly.
  18. 
      
mike_conley
  1. Great to see some documentation! Just a few suggestions below.
  2. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    Hooks aren't really there to help "customize" Extensions - they're there to give extensions a means to customize Review Board. They're one of the primary mechanisms for changing the appearance and behaviour of Review Board.
  3. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
    Show all issues
    "We briefly examine a few...demonstrage usage with sample code" - on tone, sounds a bit text-bookish.
    
    Maybe instead, simply:
    
    "The following hooks are available for extensions to use."
    
    You might want to mention that hooks need only be instantiated for Review Board to "notice" them, and that these hooks are automatically removed when the extension shuts down.
  4. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
    Show all issues
    We can make this even more general - "URLHooks are used to extend the URL patterns that Review Board will recognize and respond to."
  5. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
     
     
    Show all issues
    This last sentence is generally true for all hooks - maybe you won't need it if you add that sentence in the parent section - that hooks need only be instantiated to work, and will be automatically removed on shut down.
  6. docs/codebase/extending/extensions.txt (Diff revision 1)
     
     
    Show all issues
    Instead of "To do..." how about something like,
    
    "This is my new Dashboard page for Review Board!"
    
    That way, it makes it more clear what we're showing, and looks less like we forgot to put something in the documentation.
  7. 
      
SL
mike_conley
  1. 
      
  2. docs/codebase/extending/extensions.txt (Diff revision 2)
     
     
    Show all issues
    They don't use them as a primary mechanism, they *are* a primary mechanism.
    
    I'd be OK with "Extensions can use "hooks" as a mechanism for customizing Review Board's appearance and behavior."
    
    Also, make sure to use "Review Board". Not ReviewBoard or Reviewboard.
  3. docs/codebase/extending/extensions.txt (Diff revision 2)
     
     
     
     
    Show all issues
    Let's say this:
    
    Notice how sample_extension.urls was included in the patterns. In this case, sample_extension is the package name for the extension, and urls is the module that contains the patterns.
  4. docs/codebase/extending/extensions.txt (Diff revision 2)
     
     
     
    Show all issues
    "it then" -> "then it"
  5. docs/codebase/extending/extensions.txt (Diff revision 2)
     
     
    Show all issues
    This looks like leftover cruft to be removed.
  6. docs/codebase/extending/extensions.txt (Diff revision 2)
     
     
    Good fix!
  7. 
      
SL
mike_conley
  1. Looks good to me. Thanks Sampson!
  2. 
      
david
  1. Ship It!
  2. 
      
SL
Review request changed
Status:
Completed
Change Summary:
Pushed to release-1.7.x (d076073). Thanks!