-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This is too much work for what is already built into urllib2. We want to do: opener = urllib2.build_opener(urllib2.ProxyHandler(), urllib2.UnknownHandler(), urllib2.HTTPHandler(), ...) -
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Can you use more of a sentence form? Casing, complete sentences.
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This would have to be AttrDict, to keep with the syntax, but I'm not really a fan of this. I'd rather we either have a class with these methods or just use a dict.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Sentence form and excess whitespace. Blank line before the first comment.
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Let's be more specific. "JSON response is %s"
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Can you use keyword arguments here as well, so it's consistent with the other instances?
-
-
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Instead of "cwd", please say "the current directory"
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Blank line before each of these lines. Any time a block ends, we put a blank line after it.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) We should be explicit and check that a changenum was passed before trying to use it.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) "The current directory is not a working copy and a repository was not specified."
-
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This should be wrapped to 80 characters and should be a bit more human-readable. The error isn't really correct. You can diff by filenames. We just don't support it. The error should be more clear and say that we don't support diffs by filenames. Blank line after this line.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This should stay sys.exit(1). It's an error code. We're not exiting successfully.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Change "crap" to "stuff" or something.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) No need for getattr. This can be options[field]
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) I'd prefer this to go back to how it was. Also, You removed the blank line between the text and URL.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This isn't always correct. Maybe something like: "%prog [changenum|files]"
-
-
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This change is going to be a problem. We need to keep the existing method of assuming changenum on Perforce. Otherwise we're going to break a lot of setups. Also, the revision range stuff cannot change. For the same reasons.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) "submit changes for another user (requires special permissions)"
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This should be added by the SVNClient.
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) This should be added by the Perforce client.
-
-
-
-
/trunk/reviewboard/contrib/tools/post-review (Diff revision 1) Yes, it's used. That comment can be removed from here. Also, blank line before the comment block.
-
improve scriptability of post-commit
Review Request #212 — Created Jan. 25, 2008 and discarded
* post_review separated from __main__ to allow importing from other scripts such as post-commit hook; allows any command-line option to be set programatically in call to post_review method * adds --submit-as option * adds --repository option; allows empty basedir for repo diffs * includes json responses in debug mode * consolidates .reviewboardrc loading into one place in __main__ * allow use of -c (--revision) with svn and removed -c as a weird special case in parse_options; removed --revision-range: reviewing multiple revisions in svn makes as little sense as in p4 * generalizes and expands request field setting in mutable_request_fields * allows any (long-form) command-line option to be set in .reviewboardrc (backwards incompatibility: this means "REVIEWBOARD_URL" in .reviewboardrc should now be just "server"); if also specified on command-line, command-line takes precedence I've included this as a diff against post-review but the name needs to be changed to make it importable (hyphens are not legal in a module name). I used the obvious "post_review.py"
