Hi! I finally managed to create reviews from a post-receive git hook but now discovered a number of additional problems with this approach and ReviewBoard.
First of all it seems that reviewboard/scmtools/git.py has a mistake in GitTool._resolve_head() method. Likely (I'm not completely sure though) that the method doesn't work correctly if revision != HEAD. Actually it should return a string like <rev-id>:/path/to/a/file where ref-id is either a named ref (like 'HEAD' or 'master') or SHA-1 For HEAD it does return such a string but for other kind of refs it doesn't. The second problem is more serious, I think. It's related to ReviewBoard data model and maybe even with its workflow and thus can't be just "fixed". The post-commit/post-publish approach works like this: a user "publishes" her changes in an "official" repository (this may be a "commit" in case of a centralized VCS or a "push" of few commits in case of DVCS, doesn't matter). Then a post-publish VCS hook automatically takes these changes among their descriptions (commit messages) and makes them available as a review request (or requests). Then authorative persons look at the review requests, give their _advices_ about what to be changed in the commit(s) etc. If a change is required it's expressed as an additional commit. Should "fixing" commits be somehow related to the original review requests - this question is open for me, I have no acceptable solution. This approach is less strict and more error-prone but it allows much quicker development than "usual" "pre-publish" reviews, and I think it's more suitable for small teams residing in a single location. How do post-publish review process internals differ from pre-publish one? First of all a review tool shouldn't consider just a tip of a branch and (anonymous) changes given in an uploaded patch. Instead it has an exact "resulting" revision; a selected VCS (not necessarily git, but _any_ VCS with a notion of patchset) already has this revision checked in; both source and resulting file tree are known; changes have their author, description etc. One doesn't need to upload a patch in this case at all. In order to implement this approach in ReviewBoard (for git :) ), we have to 1. make sure that a "resulting revision" can be stored within a review request. 2. implement an additional operating mode in diffviewer when it receives repository ID, revision ID and, for certain VCS, a branch, and fetches the rest from the repository. As far as I understand ReviewRequest 'changenum' field is suitable for describing the resulting revision. Unfortunately it's an IntegerField right now and used only for Perforce. Also diffviewer has no links with "update_obj_from_changenum()" method nor uses changenum field, that's why I guess, an additional operating mode for diffviewer is to be implemented. Certainly these changes are too big to fit the reviewBoard 1.0 release process. Unfortunately I have to launch review process with the described approach in a really near future, so I can't wait till the feature is implemented in a "regular development cycle". So I'm going to start an experimental branch (actually have already started) of reviewboard and rbtools and to publish these two git repositories somewhere to let persons interested in these changes (particularly non-numeric changenum IDs) to get their hands into the code. Hopefully (indeed!) this branch will be merged back to the official ReviewBoard trunk at a later stage. Since I don't have enough experience with Reviewboard/Django internals any help with the stuff will be greatly appreciated. Maybe there's a channel (IRC, Jabber, whatever) to discuss the questions in real time? Yours respectfully, Alexey Morozov --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "reviewboard" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -~----------~----~----~----~------~----~------~--~---
