- Download Diff

Summary:

Introduce rbtools documentation

Review Request #3296 — Created Aug. 20, 2012 and submitted March 12, 2013, 10:40 p.m.

Information
Owner:	smacleod
Repository:	RBTools
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	rbtools
People:

Description

Introduce rbtools documentation

Create the configuration for the rbtools documentation and introduce
the initial documentation for the rbtools api client. This
documentation includes a small tutorial, a general overview, and
resource specific functionality.

Testing Done

Built the documentation using 'make html' and visually inspected. Quickly proofread as I went.

Issues

Description	From	Last Updated
Might as well have this fit PEP-8. Space after the :	david	Feb. 24, 2013, 2:37 p.m.
I think it would be easier to read and probably more useful if you showed this loading the diff from …	david	Feb. 24, 2013, 2:44 p.m.
?	david	Feb. 24, 2013, 2:43 p.m.
Same here re: loading from a file.	david	Feb. 24, 2013, 2:48 p.m.
Maybe show what happens if you try to publish and there's no user or group assigned?	david	March 12, 2013, 8:57 p.m.
Maybe just link "hyperlinks" to this?	chipx86	Feb. 24, 2013, 7:54 p.m.
Technically, all the keys in this would be quoted, since they do that in JSON.	chipx86	Feb. 24, 2013, 7:56 p.m.
Got an extra space in here.	chipx86	Feb. 24, 2013, 7:56 p.m.
"keyword"	chipx86	Feb. 24, 2013, 7:56 p.m.
field's	chipx86	Feb. 24, 2013, 7:56 p.m.
resource's ?	chipx86	Feb. 24, 2013, 7:57 p.m.
administrator's	chipx86	Feb. 24, 2013, 7:57 p.m.
Capitalize "Specify"	chipx86	Feb. 24, 2013, 7:57 p.m.
Resource-Specific	chipx86	Feb. 24, 2013, 7:58 p.m.
I'd maybe remove "Specific" on these sub-headers.	chipx86	Feb. 24, 2013, 7:59 p.m.
"retrieve"	chipx86	Feb. 24, 2013, 7:59 p.m.
Comma after "examples"	chipx86	Feb. 24, 2013, 8 p.m.
"API", for consistency.	chipx86	Feb. 24, 2013, 8:01 p.m.
Comma after ``uri-template``.	chipx86	Feb. 24, 2013, 8:01 p.m.
Comma after ``uri-template``	chipx86	Feb. 24, 2013, 8:01 p.m.
This looks left over from something.	chipx86	Feb. 24, 2013, 8:01 p.m.
"Review Board"	chipx86	Feb. 24, 2013, 8:02 p.m.
API?	chipx86	Feb. 24, 2013, 8:02 p.m.
We have the room, so I'd suggest we have the first parameter on the "RBClient(" line.	chipx86	Feb. 24, 2013, 8:02 p.m.
"documentation"	chipx86	Feb. 24, 2013, 8:02 p.m.
I don't think that's true anymore. It should be possible to create a review request with no repository, for file …	chipx86	Feb. 24, 2013, 8:22 p.m.
"ID"	chipx86	Feb. 24, 2013, 8:05 p.m.
Two blank lines.	chipx86	Feb. 24, 2013, 8:06 p.m.
Refs are going to use the page titles, which may not fit well in the sentence. I'd make sure to …	chipx86	Feb. 24, 2013, 8:10 p.m.
resource's	chipx86	Feb. 24, 2013, 8:10 p.m.
Comma after "attachment"	chipx86	Feb. 24, 2013, 8:11 p.m.
4 space indentation, relative to the code's indentation block.	chipx86	Feb. 24, 2013, 8:11 p.m.
Two blank lines.	chipx86	Feb. 24, 2013, 8:12 p.m.
Indentation is wonky.	chipx86	Feb. 24, 2013, 8:12 p.m.
No comma after "group"	chipx86	Feb. 24, 2013, 8:13 p.m.
Comma after "requirement"	chipx86	Feb. 24, 2013, 8:13 p.m.
Odd way to format a paragraph ;)	chipx86	Feb. 24, 2013, 8:14 p.m.
Comma after "review request'	chipx86	Feb. 24, 2013, 8:15 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 24, 2013, 5:07 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Feb. 24, 2013, 5:07 p.m.
Col: 11 E401 multiple imports on one line	reviewbot	Feb. 24, 2013, 5:07 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 24, 2013, 5:07 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Feb. 24, 2013, 5:07 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 24, 2013, 5:07 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 24, 2013, 5:07 p.m.
I feel we should just keep this as RBTools. That way it's very clear, when they see the URLs, and …	chipx86	Feb. 24, 2013, 8:17 p.m.
resource's	chipx86	Feb. 25, 2013, 3:08 p.m.
Wrong number of ===s.	chipx86	Feb. 25, 2013, 3:08 p.m.
Missing period.	chipx86	Feb. 25, 2013, 3:09 p.m.
Here too. And others.	chipx86	Feb. 25, 2013, 3:09 p.m.
Same question about the cookies.	chipx86	Feb. 25, 2013, 3:09 p.m.
Missing a trailing period.	chipx86	Feb. 25, 2013, 3:09 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 24, 2013, 8:27 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Feb. 24, 2013, 8:27 p.m.
Col: 11 E401 multiple imports on one line	reviewbot	Feb. 24, 2013, 8:27 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 24, 2013, 8:27 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Feb. 24, 2013, 8:27 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 24, 2013, 8:27 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Feb. 24, 2013, 8:27 p.m.
Same goes for here (The RBTools bit).	chipx86	Feb. 25, 2013, 3:10 p.m.
Are we instantiating the API? We're using API a lot in this paragraph - maybe we should say, "Here is …	mike_conley	March 5, 2013, 6:45 p.m.
Maybe we could show an example here?	mike_conley	March 5, 2013, 6:48 p.m.
For clarity's sake, maybe we should call this review_request, to disambiguate between review requests and WebAPI HTTP requests.	mike_conley	March 5, 2013, 6:49 p.m.
"for use" -> "to use"	mike_conley	March 5, 2013, 6:49 p.m.

Change Summary:

Added more to the overview, and about half of the tutorial.

Branch:

-	master
+	api

Diff:

Revision 2 (+663)

Show changes

	.gitignore
	docs/Makefile
	docs/conf.py
	docs/contents.txt
	docs/index.txt
	docs/api/overview.txt
	docs/api/resource-specific.txt
	docs/api/tutorial.txt

I didn't look at any of the build system stuff yet, this is just a review of the doc text.

docs/api/tutorial.txt (Diff revision 2)

Show all issues

Might as well have this fit PEP-8. Space after the :

smacleod Feb. 24, 2013, 2:54 p.m.

The proper way to make the create call now uses kwargs, so this is a non-issue now.

docs/api/tutorial.txt (Diff revision 2)

Show all issues

I think it would be easier to read and probably more useful if you showed this loading the diff from a file.

docs/api/tutorial.txt (Diff revision 2)

Show all issues

smacleod Feb. 24, 2013, 2:54 p.m.

This is just a reminder to link to the resource specific functionality documentation when I've written it. Certain resources have extra helper methods which I need to document.

docs/api/tutorial.txt (Diff revision 2)

Show all issues

Same here re: loading from a file.

docs/api/tutorial.txt (Diff revision 2)

Show all issues

Maybe show what happens if you try to publish and there's no user or group assigned?

smacleod Feb. 24, 2013, 2:54 p.m.

Hmm, that poses a bit of a problem since there is a bug in RBs API which will allow you to publish review requests without having reviewers. So, if I show what *should* happen, it will be incorrect documentation.

david Feb. 24, 2013, 3:46 p.m.
```
How about we fix that bug? :)
```

smacleod March 12, 2013, 8:57 p.m.

I'm going to drop this issue for now. I've been looking in to fixing this in RB's Web API, and it's
proving to be much more work than expected.

I'm going to work on a fix separately and not let this block the release of rbtools 0.5

Change Summary:

Update based on David's review and flesh out the api documentation some more.

Summary:

-	[WIP] RBTools Documentation
+	Introduce rbtools documentation

Description:

~		This is a sphinx configuration for RBTools documentation, and the start of documentation for the new API client.
	~	Introduce rbtools documentation
	+
	+	Create the configuration for the rbtools documentation and introduce
	+	the initial documentation for the rbtools api client. This
	+	documentation includes a small tutorial, a general overview, and
	+	resource specific functionality.

Testing Done:

Built the documentation using 'make html' and visually inspected. Quickly proofread as I went.

Branch:

-	api
+	master

Diff:

Revision 3 (+820)

Show changes

	.gitignore
	docs/Makefile
	docs/conf.py
	docs/contents.txt
	docs/index.txt
	docs/api/overview.txt
	docs/api/resource-specific.txt
	docs/api/tutorial.txt

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    docs/conf.py
  Ignored Files:
    docs/Makefile
    docs/index.txt
    .gitignore
    docs/api/overview.txt
    docs/contents.txt
    docs/api/tutorial.txt
    docs/api/resource-specific.txt

docs/conf.py (Diff revision 3)

Show all issues

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

docs/conf.py (Diff revision 3)

Show all issues

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

docs/conf.py (Diff revision 3)

Show all issues

Col: 11
 E401 multiple imports on one line

docs/conf.py (Diff revision 3)

Show all issues

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

docs/conf.py (Diff revision 3)

Show all issues

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

docs/conf.py (Diff revision 3)

Show all issues

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

docs/conf.py (Diff revision 3)

Show all issues

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

The pages themselves look pretty good, but I'm worried that just calling it "rbtools documentation" will get a lot of hits from people looking for docs on how to use rbtools. Can we call these "code base documentation" like we do for reviewboard?

smacleod Feb. 24, 2013, 6:39 p.m.

So, my intention was to have both sets be part of the same
documentation. All of the API client stuff under "api", and
the commands stuff under another folder etc.

The commands stuff is coming next, I just wrote this stuff
first.

docs/api/overview.txt (Diff revision 3)

Does this reference work?

smacleod Feb. 24, 2013, 8:24 p.m.

Yeah, the configuration is set up to check for references
in the review board documentation as well, so all these
webapi2.0 references are hooked up.

docs/api/overview.txt (Diff revision 3)
Show all issues
```
Maybe just link "hyperlinks" to this?
```

docs/api/overview.txt (Diff revision 3)

Show all issues

Technically, all the keys in this would be quoted, since they do that in JSON.

docs/api/overview.txt (Diff revision 3)
Show all issues
```
Got an extra space in here.
```
docs/api/overview.txt (Diff revision 3)
Show all issues
```
"keyword"
```
docs/api/overview.txt (Diff revision 3)
Show all issues
```
field's
```
docs/api/overview.txt (Diff revision 3)
Show all issues
```
resource's ?
```
docs/api/overview.txt (Diff revision 3)
Show all issues
```
administrator's
```
docs/api/overview.txt (Diff revision 3)
Show all issues
```
Capitalize "Specify"
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
Resource-Specific
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
I'd maybe remove "Specific" on these sub-headers.
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
"retrieve"
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
Comma after "examples"
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
"API", for consistency.
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
Comma after ``uri-template``.
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
Comma after ``uri-template``
```
docs/api/resource-specific.txt (Diff revision 3)
Show all issues
```
This looks left over from something.
```
docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
"Review Board"
```
docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
API?
```

docs/api/tutorial.txt (Diff revision 3)

Show all issues

We have the room, so I'd suggest we have the first parameter on the "RBClient(" line.

docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
"documentation"
```

docs/api/tutorial.txt (Diff revision 3)

Show all issues

I don't think that's true anymore. It should be possible to create a review request with no repository, for file attachments.

smacleod Feb. 24, 2013, 8:24 p.m.

You're correct, it will work without a repository now. I'll fix this up.

docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
"ID"
```
docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
Two blank lines.
```

docs/api/tutorial.txt (Diff revision 3)

Show all issues

Refs are going to use the page titles, which may not fit well in the sentence. I'd make sure to check all these (a simple 'make html' should generate readable output), and maybe change some to be like: :ref:`resource-specific functionality <python-api-resource-specific`.

smacleod Feb. 24, 2013, 8:24 p.m.

Yeah I've been generating as I've been writing it. This will
come out as (Links have been surrounded by []):

"This can be accomplished using the resource’s create method,
but the [Diff List Resource] has [Resource-Specific Functionality]
to make this task easier".

(Note: I updated the references name without updating its usage,
so this wasn't working properly, I've fixed it now though).

docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
resource's
```
docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
Comma after "attachment"
```

docs/api/tutorial.txt (Diff revision 3)

Show all issues

4 space indentation, relative to the code's indentation block.

smacleod Feb. 24, 2013, 8:24 p.m.

Ah, nice catch, don't know how I ended up with 5 space :P

docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
Two blank lines.
```

docs/api/tutorial.txt (Diff revision 3)

Show all issues

Indentation is wonky.

smacleod Feb. 24, 2013, 8:24 p.m.

Very wonky. Again, not sure what I was doing here haha.

docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
No comma after "group"
```
docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
Comma after "requirement"
```

docs/api/tutorial.txt (Diff revision 3)

Show all issues

Odd way to format a paragraph ;)

smacleod Feb. 24, 2013, 8:24 p.m.

I don't know what you're talking about, this is how all the
cool kids are doing it!

docs/api/tutorial.txt (Diff revision 3)
Show all issues
```
Comma after "review request'
```

docs/conf.py (Diff revision 3)

What do we want to do as far as copyright assignment? Assign it to Review Board? Leave it at you?

smacleod Feb. 24, 2013, 8:24 p.m.

I brought this up in IRC. David suggested I put myself as the
copyright, but I'm fine assigning copyright to "The Review Board
Project" or something. It's up to you guys.

docs/contents.txt (Diff revision 3)

Show all issues

I feel we should just keep this as RBTools. That way it's very clear, when they see the URLs, and it should help with any Googling.

Change Summary:

Updated based on Christian's review and fixed a couple
small issues I noticed myself.

Diff:

Revision 4 (+812)

Show changes

	.gitignore
	docs/Makefile
	docs/conf.py
	docs/contents.txt
	docs/index.txt
	docs/api/overview.txt
	docs/api/resource-specific.txt
	docs/api/tutorial.txt

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    docs/conf.py
  Ignored Files:
    docs/Makefile
    docs/index.txt
    .gitignore
    docs/api/overview.txt
    docs/contents.txt
    docs/api/tutorial.txt
    docs/api/resource-specific.txt

chipx86 Feb. 24, 2013, 9:28 p.m.

The Sphinx default config sure upsets ReviewBot, doesn't it? Maybe we should just fix these up... Though that's more just busy work right now.

docs/conf.py (Diff revision 4)

Show all issues

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

docs/conf.py (Diff revision 4)

Show all issues

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

docs/conf.py (Diff revision 4)

Show all issues

Col: 11
 E401 multiple imports on one line

docs/conf.py (Diff revision 4)

Show all issues

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

docs/conf.py (Diff revision 4)

Show all issues

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

docs/conf.py (Diff revision 4)

Show all issues

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

docs/conf.py (Diff revision 4)

Show all issues

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

docs/api/overview.txt (Diff revision 4)

Trying to decide if it's a good idea to show cookie_file=. Not sure if we want everyone out there who follows this to start copying/pasting this.

docs/api/overview.txt (Diff revision 4)
Show all issues
```
resource's
```
docs/api/resource-specific.txt (Diff revision 4)
Show all issues
```
Wrong number of ===s.
```
docs/api/resource-specific.txt (Diff revision 4)
Show all issues
```
Missing period.
```
docs/api/resource-specific.txt (Diff revision 4)
Show all issues
```
Here too. And others.
```
docs/api/tutorial.txt (Diff revision 4)
Show all issues
```
Same question about the cookies.
```
docs/api/tutorial.txt (Diff revision 4)
Show all issues
```
Missing a trailing period.
```
docs/index.txt (Diff revision 4)
Show all issues
```
Same goes for here (The RBTools bit).
```

Change Summary:

Updated based on Christian's review and fixed the pep8 issues with the auto generated configuration.

Diff:

Revision 5 (+831)

Show changes

	.gitignore
	docs/Makefile
	docs/conf.py
	docs/contents.txt
	docs/index.txt
	docs/api/overview.txt
	docs/api/resource-specific.txt
	docs/api/tutorial.txt

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    docs/conf.py
  Ignored Files:
    docs/Makefile
    docs/index.txt
    .gitignore
    docs/api/overview.txt
    docs/contents.txt
    docs/api/tutorial.txt
    docs/api/resource-specific.txt

This is really excellent! Just a few suggestions.

docs/api/overview.txt (Diff revision 5)

Show all issues

Are we instantiating the API? We're using API a lot in this paragraph - maybe we should say, "Here is an example of how to instantiate the client, and retrieve the root resource".

docs/api/overview.txt (Diff revision 5)
Show all issues
```
Maybe we could show an example here?
```

docs/api/tutorial.txt (Diff revision 5)

Show all issues

For clarity's sake, maybe we should call this review_request, to disambiguate between review requests and WebAPI HTTP requests.

docs/index.txt (Diff revision 5)
Show all issues
```
"for use" -> "to use"
```

Change Summary:

Updated based on Mike's review.

Diff:

Revision 6 (+835)

Show changes

	.gitignore
	docs/Makefile
	docs/conf.py
	docs/contents.txt
	docs/index.txt
	docs/api/overview.txt
	docs/api/resource-specific.txt
	docs/api/tutorial.txt

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    docs/conf.py
  Ignored Files:
    docs/Makefile
    docs/index.txt
    .gitignore
    docs/api/overview.txt
    docs/contents.txt
    docs/api/tutorial.txt
    docs/api/resource-specific.txt

Ship it!

Looks good. David still has that open issue that we should figure out though.

Status: Closed (submitted)

Change Summary:

Pushed to master (1d91e7f6b1076992e6248f4f0cb34d7a086cd1ec).

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

Introduce rbtools documentation

Change Summary:

Branch:

Diff:

Change Summary:

Summary:

Description:

Testing Done:

Branch:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Status: Closed (submitted)

Change Summary: