Summary

Documentation: How to Integrate ReviewUIs via Extensions

Review Request #3596 — Created Nov. 30, 2012 and submitted Dec. 1, 2012, 11:16 p.m.

Information

Owner

slchen

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

A how-to guide for creating and integrating ReviewUIs through ReviewBoard Extensions.

Content adds to http://www.reviewboard.org/docs/codebase/dev/extending/extensions/

Testing Done

Ran `make html` in reviewboard/docs/codebase/

No error output from sphinx.

Rendered HTML pass visual inspection.

Issues

Description	From	Last Updated
To reference a file, you can use :file:`extension.py`.	chipx86	Dec. 1, 2012, 8:57 p.m.
Instead of this, an alternative is to create: self.reviewui_hooks = ReviewUIHook(self, [XMLReviewUI]) in the extension's __init__. I think that's a …	chipx86	Dec. 1, 2012, 9:26 p.m.
Blank line after this.	chipx86	Dec. 1, 2012, 9:23 p.m.
While nice to demonstrate, we should keep this as simple as possible to follow and not make people wonder if …	chipx86	Dec. 1, 2012, 9:26 p.m.
Two blank lines.	chipx86	Dec. 1, 2012, 9:29 p.m.
"syntax highlighted"	chipx86	Dec. 1, 2012, 9:29 p.m.
Blank line between these.	chipx86	Dec. 1, 2012, 9:29 p.m.
And here.	chipx86	Dec. 1, 2012, 9:29 p.m.
And here.	chipx86	Dec. 1, 2012, 9:30 p.m.
I think this should just be a :file:`...`. You won't get the bold, but I don't think that's a problem.	chipx86	Dec. 1, 2012, 9:30 p.m.
Can you put a blank line around each block? Helps with readability.	chipx86	Dec. 1, 2012, 10:28 p.m.
We don't use MEDIA_URL or MEDIA_SERIAL anymore. It should all be {% static %}, in theory. Have you tried that …	chipx86	Dec. 1, 2012, 10:28 p.m.
:file:	chipx86	Dec. 1, 2012, 10:31 p.m.
:file:	chipx86	Dec. 1, 2012, 10:31 p.m.
Blank line between these.	chipx86	Dec. 1, 2012, 10:35 p.m.

docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
To reference a file, you can use :file:`extension.py`.
```

docs/codebase/extending/extensions.txt (Diff revision 1)

The issue has been resolved. Show all issues

Instead of this, an alternative is to create:

self.reviewui_hooks = ReviewUIHook(self, [XMLReviewUI])

in the extension's __init__. I think that's a better model in general for us to document, as it's less code and achieves the same thing. That can then more easily be merged into the code below.

docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line after this.
```

docs/codebase/extending/extensions.txt (Diff revision 1)

The issue has been resolved. Show all issues

While nice to demonstrate, we should keep this as simple as possible to follow and not make people wonder if they need these or what they're for.

docs/codebase/extending/extensions.txt (Diff revision 1)

I'm actually not familiar with this format. What are we doing?

slchen Dec. 1, 2012, 9:28 p.m.

This is for creating a list when items in the list need to span multiple lines in the .txt file. See Line #350 / #354 for an existing example.

docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
Two blank lines.
```
docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
"syntax highlighted"
```
docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```
docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```

docs/codebase/extending/extensions.txt (Diff revision 1)

The issue has been resolved. Show all issues

I think this should just be a :file:`...`. You won't get the bold, but I don't think that's a problem.

docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you put a blank line around each block? Helps with readability.
```

docs/codebase/extending/extensions.txt (Diff revision 1)

The issue has been resolved. Show all issues

We don't use MEDIA_URL or MEDIA_SERIAL anymore. It should all be {% static %}, in theory. Have you tried that with your extension?

slchen Dec. 1, 2012, 10:25 p.m.

Fixed: Removed {{MEDIA_SERIAL}} as it is no longer used, but kept {{MEDIA_URL}} as it's the only way to access static files uploaded through extensions.

docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
:file:
```
docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
:file:
```
docs/codebase/extending/extensions.txt (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```

Change Summary:

Fixed latest issues regarding the docs

Diff:

Revision 2 (+184)

Show changes

docs/codebase/extending/extensions.txt

Ship it!

Looks good. I made a few changes:

* Removed trailing whitespace
* ReviewUI now subclasses FileAttachmentReviewable{View}
* Added syntax highlighting for the template and JavaScript

Status:: Completed
Change Summary:: Pushed to release-1.7.x (eeddea2)