Re: Reviewboard compared to Crucible
Hi Darren, Thanks for this write-up! It helps us to know where we can improve in future versions. Some comments inline. On Wed, Apr 15, 2009 at 9:56 PM, Darren spaces...@gmail.com wrote: The Dashboard doesn't keep track of reviews that you have completed. Instead it shows all the reviews in a large list. It's quite difficult to know if you have completed a review or not by just looking at the list. I believe it just colorizes the Posted and Last Updated columns, but I believe this doesn't directly indicate the status of the review, and doesn't indicate which reviewers haven't completed. It's probably just triggered by time from when the review was created. If you click the ... button to the right of the columns, you should see a drop-down list of possible columns you can add. One of them shows whether you have commented on the review request, and will show a green comment flag for pending draft comments, a blue one for published comments, and a blue one with a checkmark if you marked Ship It! There's other columns for the number of reviews, one showing whether anyone's marked Ship It on the review request before, one showing whether there were changes since you last saw it, etc. The ... button isn't as noticeable as it could be. We'll probably look into ways to improve this in the future. Comments are hard to follow. I find that the way the list of comments gets rendered that it's hard to follow. I realize they're sequential, but I just find it confusing at times. Can you go into more detail on this one? What was expected? Having to click publish is a bit of a clunky workflow. Sometimes I forget that the green strip is there with a publish on it. I just want to say OK to a comment and have it publish. Having to click publish is an extra step I feel the tool could do without. This step is due to the e-mail functionality. One of the goals when we initially developed this was to better integrate into workflows where people would check their e-mail to read review requests. This was common at VMware (where I work) and in many open source projects. Review Board had to know when to send the e-mail. We decided the best time would be when the user actually finishes the review request. We also wanted to make sure we didn't publish comments too early. There are many times where I've made comments suggesting changes that I realized were a bad idea or already done when reading more of the diff. If these were auto-published, users would see them and waste time with an unneeded explanation. I don't know how we'd be able to get rid of the publish step without adding confusion and breaking e-mail notifications (and, in the future, other notification types that need to know when a review is complete). There's no indication as to who has completed the review, and who has yet to complete it. This would be helpful There's been a lot of demand for this. A lot of this requires company-dependent policy. Different companies have different requirements for who should review a review request. There may be a group of senior engineers that must sign off on it, or they may require 50% of the people on the target reviewer list, or 3+ people, etc. We're going to be working on coming up with a policy model that would allow for the flexibility needed to describe this after 1.0. Then we could have some UI that shows who still needs to review and who has reviewed. It can't roll up revisions. Though this is because this tool is for pre-checkins, it still makes it harder for the review to stay relevant once you have made some further changes. You would have to start another review with the new patch file, and that just makes the review dashboard messier than it already is. I'm probably confused as to what you mean here. You can upload new diffs to an existing review request. Just click Update Diff in the top-right of a review request. The post-review tool (part of RBTools) makes this really easy (and makes posting review requests themselves really easy). Some companies even have post-commit hooks that use post-review to do the work for you, though this is pretty dependent on the company. We'll continue to watch ReviewBoard closely -- it seems like it's on its way to being a solid tool. Thanks. Hope some of my answers above helped clarify a couple of things. There's a lot of good code review tools starting to spring up, which is a great thing. Review Board started as a small project to scratch an itch, and wasn't originally intended to compete against the big guys in this sector. It's not going to be for everybody, but it's always nice to hear good feedback and good criticism :) Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.review-board.org VMware, Inc. - http://www.vmware.com --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard group. To post to this group, send
Re: Poll: Now that IE8 is arriving...
On Tue April 14 2009 23:39:09 Christian Hammond wrote: 1) Your company is using IE6 and will still require IE6 even after IE8 is pushed out. 2) Your company *was* using IE6 but plans to upgrade. I am working for a large company and the official browser is still IE6. I am not expecting this will change shortly as many internal sites are optimized for it. However, many developers are using alternative browsers, mostly Firefox, but also Chrome or Opera. Thanks for your hard work. Gilles. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard group. To post to this group, send email to reviewboard@googlegroups.com To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -~--~~~~--~~--~--~---
Re: Cleartool missing + Svn access issue ?
We should certainly try to catch this. Can you file a bug on this for tracking purposes? Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.review-board.org VMware, Inc. - http://www.vmware.com On Thu, Apr 16, 2009 at 11:56 AM, Vesterbaek vesterb...@gmail.com wrote: I just had this problem on my windows as well. My problem turned out to be an old svn client used in combination with a new tortoisesvn = C:\temp\reviewboard-read-only\reviewboardsvn info svn: This client is too old to work with working copy '.'. You need to get a newer Subversion client, or to downgrade this working copy. See http://subversion.tigris.org/faq.html#working-copy-format-change for details. post-review therefore continues to try the other possible tools (and fails ungracefully with the clearcase tool as discussed) -- Jeppe --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard group. To post to this group, send email to reviewboard@googlegroups.com To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -~--~~~~--~~--~--~---
Re: Rolling out Review Board
If you happen to be using subversion, I have created a csharp post
review app that integrates into explorer nicely. I'll open source it
soon as I get a chance to clean the code up and stub out future vc
functionality. You can try the binary out by downloading from
blog.bennyland.com. (I'd give you the exact link but I'm on my phone
right now.. It's one of the last 3 posts though)
On Apr 16, 11:20 am, David B da...@ball1.net wrote:
I should add, for my distribution I also copied the GNU licence text
from the diffutils installer into a text file and included it in my
dist/ folder - although I was only testing this build process and not
actually distributing my package I wanted to make sure I followed the
rules in case it should ever be passed around within or outside our
company.
On Apr 16, 3:24 pm, David Ball da...@ball1.net wrote:
Raghu,
I can't help with bulk user loading, but on item 1
Once you have a working installation of post-review, you can use the
py2exe to create a windows executable which can be distributed to
staff (I'm assuming your users are working under windows).
I couldn't find a detailed explanation in the ReviewBoard archive, so
here's a step-by-step of how I did it (Windows XP Pro SP3, Python
2.5.4, Review Board 1.0 alpha 2)
1) Take as your starting point a working post-review.py script on a
Windows machine.
2) Download the py2exe package py2exe-0.6.9.win32-py2.5.exe
fromhttp://sourceforge.net/project/showfiles.php?group_id=15583and
install. (I assume you can use easy_install instead, but I can't make
it work with our web proxy)
3) Make sure there are no .egg files in the Python site-packages
folder (C:\Python25\Lib\site-packages). These are zipped and py2exe
can't handle them. They must be unzipped into folders of the same
name. If there are do the following:
3a) Rename by adding a .zip extension after the .egg extension.
3b) Unzip the extension to a folder with the same name as the
original .egg file.
4) In the same folder as post-review.py create a setup.py script
containing exactly the lines below. This will use py2exe to generate
the executable:
from distutils.core import setup
import py2exe
setup(
options = {'py2exe': {'bundle_files': 1}},
console = ['post-review.py'],
zipfile = None,
)
5) Run the setup.py script:
prompt python setup.py py2exe
6) The distributable package (comprising two exes and a DLL) can be
found in the new dist/ subdirectory.
The dist folder contents can be copied to client PCs. The
post-review.exe program works in exactly the same way as the original
python script it was generated from.
7) post-review requires GNU diff (at least it does when you're using
Perforce as I am). Download the GnuWin32 package
diffutils-2.8.7-1.exe
fromhttp://gnuwin32.sourceforge.net/packages/diffutils.htm
8) Run the diffutils installer. Accept the default options except
where prompted for documentation, which should be deselected as not
required.
9) Copy the following files from C:\Program Files\GnuWin32\bin to the
post-review dist/ subdirectory:
diff.exe
libiconv2.dll
libintl3.dll
10) Rename the dist/ folder to something descriptive, then ZIP it up
and distribute to users - alternatively, I was able to build a simple
Windows installer at no cost using NSIS and HM NIS Edit.
David
On Thu, Apr 16, 2009 at 2:57 PM, Raghu Kaippully raghu...@gmail.com wrote:
Hi,
I have been evaluating review board to see if we can use it within our
organization for code reviews. The initial reaction was very good and we
are nearing the end of a pilot phase where we'll roll this out to a large
number of users. I had a couple of questions:
1. Our development/build environment is perl based and nobody has python
installed in their machines. This is going to make it hard for installing
post-review. Has somebody thought about creating an installer that
bundles python, setuptools, all required modules, and post-review into
one package? I tried creating a Windows installer using NSIS and found it
quite simple. Can I conribute that to review board?
2. Is there a way to bulk-load all users and groups into the database? If
we do not do that a requester will not be able to add a reviewer in the
review request till the reviewer has logged in at least once.
Thanks in advance for the help,
Raghu
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
reviewboard group.
To post to this group, send email to reviewboard@googlegroups.com
To unsubscribe from this group, send email to
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/reviewboard?hl=en
Issue 1052 in reviewboard: Submitting a review should not trash unpublished changes
Status: New Owner: Labels: Type-Defect Priority-Medium New issue 1052 by mac4-goo...@theory.org: Submitting a review should not trash unpublished changes http://code.google.com/p/reviewboard/issues/detail?id=1052 *NOTE: Do not post confidential information in this bug report.* What's the URL of the page containing the problem? /r/## What steps will reproduce the problem? 1. Edit the description. Do not publish it. 2. Click Close - submit What is the expected output? What do you see instead? The user should either be prompted to publish changes or the changes should simply be saved. Whats actually happening is the changes disappear. It is a reasonable use case that a user would want to add some metadata when marking the review as subbmited. The change number, date deployed, whatever. The current workflow requires them to publish, which means there is email sent to team members which they actually don't need to get. What operating system are you using? What browser? Shouldn't matter. Linux RB host. mac os client w/ firefox. Please provide any additional information below. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1053 in reviewboard: 404 when adding comments to new unpublished review request
Status: New Owner: Labels: Type-Defect Priority-Medium New issue 1053 by vesterbaek: 404 when adding comments to new unpublished review request http://code.google.com/p/reviewboard/issues/detail?id=1053 What's the URL of the page containing the problem? http://demo.review-board.org/r/1687 (unpublished review) What steps will reproduce the problem? 1. Create new review request (I used post-review to demo.review-board.org) 2. View diff 3. Try to comment some lines What is the expected output? What do you see instead? Code can be commented before review is published (e.g. to give extra info to reviewers) The comment box appears, comment is visible in the diff but it generates a red error box in the top of the page: Page not found (404) Request Method: POST Request URL:http://demo.review- board.org/api/json/reviewrequests/1687/diff/0/file/1904/line/34/comments/ No FileDiff matches the given query. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1054 in reviewboard: reviewboard doesn't escape html
Status: New Owner: Labels: Type-Defect Priority-Medium New issue 1054 by darkness: reviewboard doesn't escape html http://code.google.com/p/reviewboard/issues/detail?id=1054 *NOTE: Do not post confidential information in this bug report.* What steps will reproduce the problem? 1. edit a review and add some javascript code: scriptalert(document.cookie)/script 2. publish the review 3. the script is executed and there's no way to remove it from the page. What is the expected output? What do you see instead? all html should be escaped from user input to prevent css attacks. What operating system are you using? What browser? ff3, osx -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1055 in reviewboard: Interdiff of deleted files broken.
Status: New Owner: Labels: Type-Defect Priority-Medium New issue 1055 by josh.hamacher: Interdiff of deleted files broken. http://code.google.com/p/reviewboard/issues/detail?id=1055 *NOTE: Do not post confidential information in this bug report.* What's the URL of the page containing the problem? https://reviewboard.domain.com/r/298/diff/1-2/#index_header What steps will reproduce the problem? 1. Create a changelist (we use Perforce) that includes files open for deletion. 2. Publish the review. 3. Update the review. 4. Look at the interdiff view. The diff for the files that were opened for delete is this text: Diff currently unavailable. Error: Internal error. Unable to locate file record for filediff 2808 What is the expected output? What do you see instead? Since there was no change in the deleted file between revisions 1 and 2 I wouldn't expect the interdiff to show those files. What operating system are you using? What browser? Shouldn't matter. We're using the latest beta. Please provide any additional information below. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1054 in reviewboard: reviewboard doesn't escape html
Comment #1 on issue 1054 by darkness: reviewboard doesn't escape html http://code.google.com/p/reviewboard/issues/detail?id=1054 also, any js added seems to break the functionality of the page. i now can't autocomplete any names on the review i added js to (for vmw folks see review 12439 on reviewboard-qa). -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1054 in reviewboard: reviewboard doesn't escape html
Comment #2 on issue 1054 by darkness: reviewboard doesn't escape html http://code.google.com/p/reviewboard/issues/detail?id=1054 scratch commment #1, the ac problem has nothing to do with this (my group doesn't exist on the new reviewboard server). -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 472 in reviewboard: HGTool does not support git style diff
Comment #10 on issue 472 by dimit...@glezos.com: HGTool does not support git style diff http://code.google.com/p/reviewboard/issues/detail?id=472 The `hg export --git` command includes the revision information in the diff and could be pulled like ivazqueznet mentions. $ hg export --help hg export [OPTION]... [-o OUTFILESPEC] REV... dump the header and diffs for one or more changesets Print the changeset header and diffs for one or more revisions. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1054 in reviewboard: reviewboard doesn't escape html
Updates: Status: Duplicate Mergedinto: 894 Comment #3 on issue 1054 by chipx86: reviewboard doesn't escape html http://code.google.com/p/reviewboard/issues/detail?id=1054 This has been fixed for a while in SVN, and will be made available on the next server upgrade. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 894 in reviewboard: HTML/angle brackets in description are treated as HTML
Comment #5 on issue 894 by chipx86: HTML/angle brackets in description are treated as HTML http://code.google.com/p/reviewboard/issues/detail?id=894 Issue 1054 has been merged into this issue. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
Issue 1056 in reviewboard: Review button shouldn't leave the diff view
Status: New Owner: Labels: Type-Defect Priority-Medium New issue 1056 by jamesdlin: Review button shouldn't leave the diff view http://code.google.com/p/reviewboard/issues/detail?id=1056 *NOTE: Do not post confidential information in this bug report.* What's the URL of the page containing the problem? What steps will reproduce the problem? 1. View the diff to some review request. 2. Click the Review button (because as I'm reading the diff, I want to make some general comment about, say, the design). 3. Click the Save button. What is the expected output? What do you see instead? Reviewboard leaves the diff view and throws me to the review request's main page. Consequently, I've now lost my place in the diff. IMO, this should happen only when clicking Publish, not Save. What operating system are you using? What browser? Firefox 3.0.8 on Windows XP x64 SP2. Please provide any additional information below. -- You received this message because you are listed in the owner or CC fields of this issue, or because you starred this issue. You may adjust your issue notification preferences at: http://code.google.com/hosting/settings --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups reviewboard-issues group. To post to this group, send email to reviewboard-issues@googlegroups.com To unsubscribe from this group, send email to reviewboard-issues+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard-issues?hl=en -~--~~~~--~~--~--~---
