Re: How to connect comments and changes?
Hi Scott, That's exactly the functionality I was looking for. Great! Now even my review request pages provide a good overview. :-) It would be helpful to underline this feature in the documentation / user guide / Commenting on Lines: Commenting on Lines To comment on a line on a diff, simply click the line number. A comment dialog will appear giving you a text entry for writing your comment. When you’re done, you can click Save to save the comment. Furthermore you can assign a comment to multiple code lines. This option is especially useful to provide additional code context to discussions as all commented code will appear on the review request page. To create a multiple line comment go to the line number column and hold down the mouse to mark multiple lines. Do not exit the line number column while marking the lines. Currently I don't have access to the RB svn repository. I will check if the documentation is contained in the repository and submit a documentation patch later. Philipp On Aug 11, 6:04 pm, Scott Quesnelle scott.quesne...@gmail.com wrote: Hey Philipp, When you post a comment, instead of clicking on a single line, you can click and drag over a group of lines, and then that whole set of lines shows up for extra context. We have found its useful to have that functionality since it allows the user to tailor the amount of context necessary for the comments. Scott On Tue, Aug 11, 2009 at 10:42 AM, Philipp Henkel philipp.hen...@gmail.comwrote: Hi Christian, Thanks for pointing out that the diff viewer is not the right place for tracking changes and discussions. I understand that tracking comments across revisions is difficult. However, I was asking because some of the commercial review tools provide such functionality. If it is recommended to use mainly the review request page I have another first-time user question: Would it be possible to show more code context on this page? One line of code is fairly small in the context of a big review session. Best regards, Philipp On Aug 7, 10:37 pm, Christian Hammond chip...@chipx86.com wrote: Hi Philipp, All the discussion on a series of changes takes place on the review request's page. You shouldn't really use the diff viewer for this. Migrating comments across revisions of a diff is incredibly hard and impossible to do successfully, consistently. You can't guarantee the lines will match up in any way, and you could end up dropping comments or moving them onto the wrong lines. They also might just be irrelevant. We don't have any plans at all to even attempt something like this, as that's really what the review request page is for. You can see a timeline of all the reviews, the affected code, and you can always click on the header for the block of code to jump to the right place in the diff. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com On Fri, Aug 7, 2009 at 6:29 AM, Philipp Henkel philipp.hen...@gmail.com wrote: Hello, I'm new to Review Board and I would like to use it in combination with Perforce and post-review. There seems to be no strong connection of comments and code changes. In Diff View comments seem to be bound to single diffs. When you add a new diff it is difficult to match old comments and new changes because you don't see the previous comments. Questions like Is every issue addressed? or What is the reason for that change? cannot be answered immediately. You always have to go back to one of the previous diffs and search for the discussion. Our current review work flow is like this: 1. Create new request: add a large diff using post-review (might be a complete new feature) 2. Reviewer and Coder add comments to this diff 3. Coder applies changes and adds an updated diff using post-review 4. Reviewer is satisfied - go to 5 or next iteration is started - go to 2. 5. Review done I know that RB is optimized for pre-commits and I assume that those pre-commits reviews tend to be much smaller than post-commits ones. However, perhaps there is a simple solution to my problem if I slightly change the work flow. Did I overlook something? Is it possible to show older comments in the latest diff? Would this be a nice feature? Best regards, Philipp --~--~-~--~~~---~--~~ 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
Roadmap?
I might have missed it, but I can't seem to find a roadmap on your web site. Is there a planned timeline for the various releases? I am anxiously waiting for some of the features coming in 2.0. Specifically, issue 421 on the bug tracking list. My users never close their review requests without much nagging and it would be nice to have this daily nag feature. --~--~-~--~~~---~--~~ 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: improvements
thanks. 1. vhost: I'll have a play and see if I can fix it, if so I'll send you a patch. 2. Lucene: I thought it had python bindings already, but I can't find much info on the web. I may have a look at converting the perl or php bindings to python when I have some time. 3.egg cache, I used the conf/apache_modpython.conf file. (I had to copy it as I use CentOS - you copy the file into /etc/httpd/conf.d to get apache to pick it up). I can't find what version of RB I am using in the installed code (I thought it'd be the latest though). 4. LDAP... I don't know python, so I can't help much. The idea behind the proxy user is that you log on as that user and perform a search for the user you want to log on as. If that succeeds, you use the same connection to bind as the user with the user's password. Lots of people on this thread have wanted ldap, might I point you in the direction of http://www.python-ldap.org/docs.shtml Andy --~--~-~--~~~---~--~~ 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: improvements
On Aug 4, 9:27 pm, Christian Hammond chip...@chipx86.com wrote: Hi, On Wed, Jul 29, 2009 at 3:58 AM, gbjbaanb andy.bolstri...@gmail.com wrote: Hi everyone. I'm a very new user of ReviewBoard, I've just set it up, and as you asked for information on how I intend to use it, and suggestion for improvement... I'd thought I'd post now while my initial thoughts are still fresh. Feedback is always welcome! So far... I've got it set up and it looks good, there were a few pains getting going though: 1. in my setup, I only have a single DNS entry for the webserver. I'm using it in a corporate environment, and I don't want to set up a new box just for RB, so I thought I'd install it on the existing web server (which runs a few other development-related sites such as mantis). Unfortunately the installer assumes that it will be running as a vhost, and takes over the entire web on port 80. If this could be changed to a directory instead of a vhost, I think that would be a good improvement to make. Currently I have it running on a different port, as I cannot affect the other sites (and I'm not too hot on mod_python requirements) Most of the time, a vhost suffices and is the easiest to work with. It's a sample configuration file, so we expect people who are using more custom setups to customize the config file. We can probably look into generating other forms of config files in the future, but it's the kind of thing we'd prefer to take a patch for. 2. When installing, the dependancy on PyLucene made me decide not to bother with search :) Would it be a good idea to try using Clucene instead as it might be easier to get installed, and possibly faster/ efficient to run? PyLucene is definitely a pain to install. I wish there was something we could do about that.. Clucene isn't an option, because it's, well, not Python. We need to be able to hook into it, and need Python bindings for this, which is what PyLucene is. We may look into alternative, pure-Python search engines that we can use in the future, though they will almost surely be slower than PyLucene. Maybe that's okay though 3. When I installed it, I immediately had a 'taking a nap' error. Looking at the logs showed a problem with the python egg cache. Setting the environment variable inside the vhost directive fixed this, but it was an annoyance that it didn't work out of the box. Also, the error message could be worded better, 'taking a nap' suggests a delay, not a fatal error! Which config file did you use? The python egg cache should have been set correctly in generated config files. The errors are something we're working to improve. That particular error is from back in the day when Review Board was used in only 2 places: review-board.org and VMware. 4. LDAP support. I access active directory from a linux box, no problem with that except that the corporate AD doesn't allow anonymous reads. Most other sites I use from my linux server (eg wikipedia, mantis) allow you to set a ldap proxy user to connect with. That user then attempts to search for the user specified and logs you in. I couldn't see any option for this in the web UI, is it supported? I don't know a lot about LDAP. Our support comes from third parties who need LDAP support. So, I can't really answer this, except to say that I don't believe we have support for a proxy user, and we'd need a patch to support it. thanks. 1. vhost: I'll have a play and see if I can fix it, if so I'll send you a patch. 2. Lucene: I thought it had python bindings already, but I can't find much info on the web. I may have a look at converting the perl or php bindings to python when I have some time. 3.egg cache, I used the conf/apache_modpython.conf file. (I had to copy it as I use CentOS - you copy the file into /etc/httpd/conf.d to get apache to pick it up). I can't find what version of RB I am using in the installed code (I thought it'd be the latest though). 4. LDAP... I don't know python, so I can't help much. The idea behind the proxy user is that you log on as that user and perform a search for the user you want to log on as. If that succeeds, you use the same connection to bind as the user with the user's password. Lots of people on this thread have wanted ldap, might I point you in the direction of http://www.python-ldap.org/docs.shtml Andy --~--~-~--~~~---~--~~ 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 -~--~~~~--~~--~--~---
Trouble posting reviews since 1.0.1 upgrade
Hello everybody,
since upgrading from 1.0rc2 to 1.0.1 I have trouble posting reviews to
RB using a custom post-review implementation. The problem is that the
JSON response does not contain too many error details, just:
{fields: {path: [substring not found]}, stat: fail, err:
{msg: One or more fields had errors, code: 105}}
The log level is set to DEBUG, but nothing meaningful shows up in the log.
I am slightly lost how to determine the root cause of this issue. Any
advice how to diagnose it is highly appreciated.
Thanks,
Thilo
--~--~-~--~~~---~--~~
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
-~--~~~~--~~--~--~---
[ClearCase] cat: /usr/cc_storage/unix_STREAM/.../myjava.sql@@/main/STREAM/2: No such file or directory\n
Hi,
I wonder how reviewboard fetches file versions from ClearCase. Now I
have strange error from /api/json/reviewrequests/74/diff/new/:
{fields: {path: [cat: cat: /usr/cc_storage/unix_STREAM/.../
myjava.sql@@/main/STREAM/2: No such file or directory\n]}, stat:
fail, err: {msg: One or more fields had errors, code: 105}}
It seems that reviewboard (on server side) doesn't use cleartool to
fetch version of file. This can be seen from scmtools/clearcase.py:
class ClearCaseClient:
def __init__(self, path):
self.path = path
def cat_file(self, filename, revision):
p = subprocess.Popen(
['cat', filename],
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
close_fds=(os.name != 'nt')
)
contents = p.stdout.read()
errmsg = p.stderr.read()
failure = p.wait()
if not failure:
return contents
if errmsg.startswith(fatal: Not a valid object name):
raise FileNotFoundError(filename)
else:
raise SCMError(errmsg)
Is it mistake or my misunderstanding?
Thank you for your help!
--~--~-~--~~~---~--~~
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: Roadmap?
Hi Isaac, There's on up-to-date schedules available. We don't have a timeline for 2.0 yet, and are still not 100% sure whta's going to make it into 2.0 and what's going to go into a future release. A rough timeline is as follows: * Review Board 1.1 will enter beta cycle probably in about 2 months. Hopefully, the final release will be by the end of the year. * Review Board 1.2 will enter development after 1.1 is out, and will contain a few features, but we don't have that nailed down yet. That will hopefully be a short release, maybe 4 or 5 months. * Review Board 1.5 or 2.0 will be next, depending on what we decide to call it. This will have extension support, and that will take us a good deal of time to develop and to get the codebase in good shape for people to develop extensions. So, this release will be longer. As for issue 421, there's no reason someone couldn't write this today. It's really something that would be a scheduled task, ran outside of Review Board itself, much like our search indexing. I think this could be bumped to 1.2. Maybe even 1.1, but someone would need to supply the patch, probably, as I have a lot personally that I'm working on for this release. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.review-board.org VMware, Inc. - http://www.vmware.com On Wed, Aug 12, 2009 at 7:06 AM, Isaac Wagner geekyis...@gmail.com wrote: I might have missed it, but I can't seem to find a roadmap on your web site. Is there a planned timeline for the various releases? I am anxiously waiting for some of the features coming in 2.0. Specifically, issue 421 on the bug tracking list. My users never close their review requests without much nagging and it would be nice to have this daily nag feature. --~--~-~--~~~---~--~~ 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: View Diff shows blank screen
Glad you got it working! Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.review-board.org VMware, Inc. - http://www.vmware.com On Wed, Aug 12, 2009 at 5:23 AM, Geetanjali geetanjali...@gmail.com wrote: I could finally resolve the issue and get RB running :) pysvn was crashing as I was using an older version of subversion client. After upgrading it to 1.4.6 and rebuilding pysvn, I am able to work with reviewboard. Thanks for the quick responses. Regards, Geetanjali On Aug 11, 12:26 pm, Christian Hammond chip...@chipx86.com wrote: Almost sounds like PySVN is crashing or something. Otherwise, some exception would be shown, and you'd see *something* on the screen. Which distro is this, and what method did you use for installing both libsvn and pysvn? Are you using http, https, svn://, or svn+ssh:// for the SVN repository? One thing to try is to open up a python console and type the following: import pysvn client = pysvn.Client() # If you have a username and password configured, specify the lines below. Otherwise, don't type them. client.set_default_username(YOUR_REPO_USERNAME) client.set_default_password(YOUR_REPO_PASSWORD) # Specify the full path to the file below, with the repository path included. data = client.cat(YOUR_FILE_PATH_WITH_REPO, REVISION_TO_FETCH) print data If this crashes, you'll see some sort of useful error. Ideally, you should use the same data you're giving to Review Board. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com On Mon, Aug 10, 2009 at 4:34 AM, Geetanjali geetanjali...@gmail.com wrote: After some debugging I found that the execution does not continue after the following line in scmtools/svn.py - data = self.client.cat(normpath, normrev) The variables normpath and normrev contain correct values and if the same values are used in a test program to get the data using pysvn, I get the output on screen. I am not sure what the issue is when using pysvn with reviewboard. Regards, Geetanjali On Aug 10, 11:13 am, Geetanjali geetanjali...@gmail.com wrote: Hi Christian, Thanks for the response. I am using firefox with firebug plugin. There is no error displayed on the page. Also, the HTML source is blank. I guess there is some exception occurring at the Server and the HTML is not getting generated. However, I am not able to debug any further. Where can I see the logs ? Do I need to change any settings to see the logs ? Thanks, Geetanjali. On Aug 8, 2:53 am, Christian Hammond chip...@chipx86.com wrote: Does the JavaScript console show any errors for the page? Is this with Firefox? Installing the Firebug plugin may provide some more useful information. Also, is there any content on the page at all? If you view the source, is it empty? Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com On Thu, Aug 6, 2009 at 4:58 AM, Geetanjali geetanjali...@gmail.com wrote: Hi, I have installed RB on linux for a project using subversion. After creating a review request, on clicking View Diff, I get a blank screen. The URL points to - http://url to reviewboard/r/5/diff/ #index_header There are no error messages in the logs. I have installed pysvn 1.6.3 Can you please suggest any pointers to solve this issue. Thanks a lot. --~--~-~--~~~---~--~~ 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: [ClearCase] cat: /usr/cc_storage/unix_STREAM/.../myjava.sql@@/main/STREAM/2: No such file or directory\n
I think the person who contributed ClearCase support on the server expected
that the repository would be locally checked out with the path given. We'll
certainly take a patch to use cleartool to fix this.
We don't have access to ClearCase, so we can't do a lot of testing
ourselves.
Christian
--
Christian Hammond - chip...@chipx86.com
Review Board - http://www.review-board.org
VMware, Inc. - http://www.vmware.com
On Wed, Aug 12, 2009 at 10:56 AM, MiZhKa miz...@gmail.com wrote:
Hi,
I wonder how reviewboard fetches file versions from ClearCase. Now I
have strange error from /api/json/reviewrequests/74/diff/new/:
{fields: {path: [cat: cat: /usr/cc_storage/unix_STREAM/.../
myjava.sql@@/main/STREAM/2: No such file or directory\n]}, stat:
fail, err: {msg: One or more fields had errors, code: 105}}
It seems that reviewboard (on server side) doesn't use cleartool to
fetch version of file. This can be seen from scmtools/clearcase.py:
class ClearCaseClient:
def __init__(self, path):
self.path = path
def cat_file(self, filename, revision):
p = subprocess.Popen(
['cat', filename],
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
close_fds=(os.name != 'nt')
)
contents = p.stdout.read()
errmsg = p.stderr.read()
failure = p.wait()
if not failure:
return contents
if errmsg.startswith(fatal: Not a valid object name):
raise FileNotFoundError(filename)
else:
raise SCMError(errmsg)
Is it mistake or my misunderstanding?
Thank you for your help!
--~--~-~--~~~---~--~~
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: Trouble posting reviews since 1.0.1 upgrade
Hello Christian, On Wednesday 12 August 2009 22:25:03 Christian Hammond wrote: There's another thread on this problem as well. Are you using ClearCase? no, this happens with a proprietary SCM, for which I added support to RB and post-review. So, I cannot rule out that my scm implementation is subtly broken and the error was masked in versions up to 1.0rc2. Regards, Thilo --~--~-~--~~~---~--~~ 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: Trouble posting reviews since 1.0.1 upgrade
Given that this error has been seen on both yours and ClearCase, it's possible it's somewhere internal to Review Board. Try this... Edit reviewboard/webapi/json.py, scroll down to the new_diff function, and locate the except Exception, e: line. Remove that whole block for that exception (including the except: line) and then restart your web server. If you run post-review with --debug again, you should see an error page instead (assuming you still have DEBUG set to True). I imagine this is where the error is being caught and turned into a field error. I'd really like to know what the results are of this test, and where the problem is. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.review-board.org VMware, Inc. - http://www.vmware.com On Wed, Aug 12, 2009 at 1:41 PM, Thilo-Alexander Ginkel th...@ginkel.comwrote: Hello Christian, On Wednesday 12 August 2009 22:25:03 Christian Hammond wrote: There's another thread on this problem as well. Are you using ClearCase? no, this happens with a proprietary SCM, for which I added support to RB and post-review. So, I cannot rule out that my scm implementation is subtly broken and the error was masked in versions up to 1.0rc2. Regards, Thilo --~--~-~--~~~---~--~~ 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: Trouble posting reviews since 1.0.1 upgrade
It sounds like it's actually using the ClearCase SCM instead of your own. Are you sure your repository entry is still mapping to your custom SCM? In 1.0, we accidentally left out the database entry for the ClearCase SCM, and added it back. When you did the rb-site upgrade, it probably replaced your entry with the ClearCase one. This is pretty much the problem with using the database for these registrations. I'm looking into doing some introspection to find all the available SCMs instead of looking them up from a database so that this won't be an issue in the future. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.review-board.org VMware, Inc. - http://www.vmware.com On Wed, Aug 12, 2009 at 2:27 PM, Thilo-Alexander Ginkel th...@ginkel.comwrote: On Wednesday 12 August 2009 22:55:44 Christian Hammond wrote: Given that this error has been seen on both yours and ClearCase, it's possible it's somewhere internal to Review Board. Try this... Edit reviewboard/webapi/json.py, scroll down to the new_diff function, and locate the except Exception, e: line. Remove that whole block for that exception (including the except: line) and then restart your web server. If you run post-review with --debug again, you should see an error page instead (assuming you still have DEBUG set to True). I imagine this is where the error is being caught and turned into a field error. I'd really like to know what the results are of this test, and where the problem is. Strangely, RB seems to route the diff upload through the ClearCase SCM. The error happens at: [...] reviewboard\scmtools\core.py:51 reviewboard\scmtools\clearcase.py:43 reviewboard\scmtools\clearcase.py:81: elem_path = elem_path[elem_path.rindex(vobs/)+5:] Vars: drive '' elem_path u'/MessageConfiguration.java' path u'/MessageConfiguration.java' Thanks, Thilo --~--~-~--~~~---~--~~ 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: Discussion about policy support.
Hi Eduardo,
My biggest concern at this point is adding another dependency, especially
one that isn't pure-Python and easy_install'able. We have a lot of
dependencies today, some optional, some required. Those that are required
and pure Python are easy. We can just add them to Review Board's dependency
list. However, those that require some C components or complex installation
(such as PyLucene) can be very difficult depending on platform, distro, etc.
I wonder if we truly need that level of complexity for the implementation of
this. Having the predicate editor is good, but I don't think we need any
sort of complex DSL backing it necessarily.
Here's what I sort of envisioned:
1) Have a format stored in the database to represent the predicates. We
could either do something like JSON:
{'name': 'My Predicate',
'conditions': (
{'op': 'AND', 'conditions': (
{'op': 'GT',
'field': 'shipit_count',
'value': 3},
{'op': 'IN',
'field': 'reviewers.group',
'value': 'senior-engineering'},
})
}
... or something simpler, like:
(shipit_count 3 AND reviewers.group IN senior-engineering)
I prefer the former if the user will never see it, or the latter if they
will. One could argue that a predicate editor should always be available but
never required, if the user wanted to do more complex expressions involving
nested parens and such.
2) Once we have the above, which would work for many standard cases, we
could implement more custom behavior by way of custom Python code. This
would be after extensions comes in, probably, but what I envisioned was to
have an extension register new, custom checks that could be referenced along
with or instead of an expression above. This would allow for even more
custom behavior than what a DSL would allow for.
For example, imagine a company wants to have a policy that you can only
commit a review request if you have 3 ship-its *and* the branch isn't
locked, *and* the change is committed. You might have an extension that does
something like:
class MyPolicyExtension(Extension):
def __init__(self):
registerPolicyOp(BRANCH_LOCKED, self.__isBranchLocked)
registerPolicyOp(CHANGE_COMMITTED, self.__isChangeCommitted)
def __isBranchLocked(self, review_request, values):
# Do some check here to determine if values[0] (branch_name) is
locked or not, and return a bool.
def __isChangeCommitted(self, review_request, values):
# Do some check here to determine if the change represented by
review_request is committed
# or not, and return a bool.
Then you could modify your expression to be:
(shipit_count 3 AND NOT BRANCH_LOCKED(review_request.branch) AND NOT
CHANGE_COMMITTED)
That's just a rough idea of how it could look. The idea, though, is that
you'd have some nice, simple, storable, easy to parse format (whether
something like the expression above or JSON) that represented these
expressions that, by default, knew about field names and basic operators,
and then you could extend that with custom policy extensions, which any
extension out there could register. These custom ops would appear in the
predicate editor and everything.
This gets really powerful. Imagine a BuildbotExtension that allows for
automatically posting a change to a build infrastructure, and registering a
policy operator allowing the admin to prevent changes from being marked
submitted if the build failed.
Nice thing about all this is that we can do it with custom code only,
without adding any more dependencies. It can be completely under our
control.
Some food for thought :)
Christian
--
Christian Hammond - chip...@chipx86.com
Review Board - http://www.review-board.org
VMware, Inc. - http://www.vmware.com
On Tue, Aug 11, 2009 at 2:01 PM, Eduardo Felipe
eduardofelip...@gmail.comwrote:
Hi there!
I'm Eduardo and I'm working on Reviewboard for Google Summer of Code.
My proposal included policy support, and as I was discussing it with
Christian on IRC when he asked me to open up the discussion to the
entire community.
So, to quote (parts of) my proposal:
quote
During the deployment of ReviewBoard in my last employer we've had to
establish that no code is good until it was reviewed by at least three
programmers, two being senior. Since currently there is no way to
specify this in ReviewBoard reviews sometimes ended Close as submitted
without the minimum reviews rule. Talking to everyone about it solved
the problem, but in a large organization there should be a way to
prevent users from breaking the rules.
As such I propose, based on the suggestion on the Wiki, to create an
admin module to allow arbitrary policies for common actions. In this
way a rule could be created for anything, from reviewing, to updating
a diff, to marking it Ship it! or allowing it to be closed, deleted
or submitted,
Using post-review with git and gitosis
Hi,
We're evaluating Review Board and we're having trouble using post-review
with git repositories. Basically, we keep getting an error that looks like
this:
HTTP POSTing to http://reviews.ninginc.com/api/json/reviewrequests/new/:
{'repository_path': 'test-bazel', 'submit_as': 'davidf'}
Error creating review request: The repository path specified is not in the
list of known repositories (code 206)
The repository that the post is being sent from is a clone of a repository
that lives on a gitosis server (which happens to be the same box as the
Review Board server). The repository is (as far as I know) correctly
configured in Review Board; there's a 'local' clone of the repository that
points to the 'master' repository (which, as I said, is on the same
machine).
Any clues? Any suggestions as to what we could try? I've tried a couple of
different variants for the Mirror path for the repository with no joy.
Thanks,
Kate Ebneter
--~--~-~--~~~---~--~~
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: Sourcegear Vault
More: I traced the source of that parameter to get_original_file() in diffutils.py. When I bypass the cache lookup and just return the result of the fetch_file() sub-function I can display diffs properly, although response is very slow as expected. Any ideas where I should look to determine why the cache contents would be wrong? (I do have the memcached server installed) Thanks! On Aug 12, 5:54 pm, schuijo schu...@gmail.com wrote: I think I am very close now, but I have a problem in diffutils.py. The second parameter in the parse() function (file I believe) contains some invalid data, where does this get populated? Thanks! On Aug 11, 2:29 pm, Christian Hammond chip...@chipx86.com wrote: Hi, Path is the path to the file in the repository. If the path in the diff is always going to be absolute, then you can completely ignore base_path and just use path. We use base_path for Subversion, where the filenames in the diff are relative to the current directory rather than the root of the repository. We then append path to base_path to generate that absolute path. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com On Tue, Aug 11, 2009 at 10:54 AM, schuijo schu...@gmail.com wrote: Christian, Ok...I think I've severely bitten off more than I can chew, but I'm trying to forge my way through this. I've been modifying post-review to add Vault support, and appear to have it working to the point where it seems to be attempting to contact the Vault server while uploading the diff. The point I'm a little lost/confused on is how to represent Vault in the RepositoryInfo class. What exactly are path and base_path and how are they used? (hopefully this will help me to determine what need to be populated in there for Vault) Thanks! On Jul 28, 4:05 pm, Christian Hammond chip...@chipx86.com wrote: There are some threads on the mailing list about doing this, but they're not exactly step-by-step tutorials. The best reference right now is the scmtools/*.py files. Basically, you'll create a subclass of SCMTool that does the following: 1) Grabs a file from a repository, given a file path and revision. 2) Provide a DiffParser subclass that handles pulling out filenames and revisions and any other necessary data from a diff (most of the code for all this is common, so you just hook into things -- see the other files for examples). 3) If Vault has a concept of server-side changesets (you register a changeset with a description, and other data, and the server always knows what you have checked out on the client) then you'll need to implement get_changeset(). So the general model is that this code will have three classes: 1) VaultTool 2) VaultDiffParser 3) VaultClient VaultTool will be a subclass of SCMTool and will be what Review Board talks to. VaultDiffParser will be a subclass of DiffParser and will override functions to parse revision info out of a diff. VaultClient will be a wrapper around the command line tool, which VaultClient will talk to. Now, let's talk diffs. Many revision control systems provide tools that generate diffs unsuitable for Review Board, and sometimes we have to work around them. If vault's tool generates a diff containing revision information for a file that can be used to pull data from the repository, then we're good. If not, you'll need to implement this in post-review. Basically, generate a custom diff, or post-process the generated diff, to stuff revision information in. Then VaultDiffParser will parse that out. You'll see this done for Perforce and many others in post-review. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com On Tue, Jul 28, 2009 at 1:42 PM, schuijo schu...@gmail.com wrote: I couldn't locate a Python API for Vault, but they do have a command line tool. Is there a document or thread detailing how to implement the interface? Thanks! On Jul 28, 3:00 pm, Christian Hammond chip...@chipx86.com wrote: Not to my knowledge. Some have talked about wanting it, but I don't know if anyone's working on it. If you'd like to work on it, we'll provide guidance. The only real requirement is that there's a command line tool or Python API for grabbing a file from a remote server. Everything else can be done by us. It does help, though, if the tool/API can generate a suitable diff, or at least one we can post-process. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com
Re: Sourcegear Vault
Hi, It's possible that when you use the cache function, it's returning cached data from some older, broken attempt. If you reenable the caching and then fully clear the cache and try again, does it work? Christian On Wednesday, August 12, 2009, schuijo schu...@gmail.com wrote: More: I traced the source of that parameter to get_original_file() in diffutils.py. When I bypass the cache lookup and just return the result of the fetch_file() sub-function I can display diffs properly, although response is very slow as expected. Any ideas where I should look to determine why the cache contents would be wrong? (I do have the memcached server installed) Thanks! On Aug 12, 5:54 pm, schuijo schu...@gmail.com wrote: I think I am very close now, but I have a problem in diffutils.py. The second parameter in the parse() function (file I believe) contains some invalid data, where does this get populated? Thanks! On Aug 11, 2:29 pm, Christian Hammond chip...@chipx86.com wrote: Hi, Path is the path to the file in the repository. If the path in the diff is always going to be absolute, then you can completely ignore base_path and just use path. We use base_path for Subversion, where the filenames in the diff are relative to the current directory rather than the root of the repository. We then append path to base_path to generate that absolute path. Christian -- Christian Hammond - chip...@chipx86.com Review Board -http://www.review-board.org VMware, Inc. -http://www.vmware.com On Tue, Aug 11, 2009 at 10:54 AM, schuijo schu...@gmail.com wrote: Christian, Ok...I think I've severely bitten off more than I can chew, but I'm trying to forge my way through this. I've been modifying post-review to add Vault support, and appear to have it working to the point where it seems to be attempting to contact the Vault server while uploading the diff. The point I'm a little lost/confused on is how to represent Vault in the RepositoryInfo class. What exactly are path and base_path and how are they used? (hopefully this will help me to determine what need to be populated in there for Vault) Thanks! On Jul 28, 4:05 pm, Christian Hammond chip...@chipx86.com wrote: There are some threads on the mailing list about doing this, but they're not exactly step-by-step tutorials. The best reference right now is the scmtools/*.py files. Basically, you'll create a subclass of SCMTool that does the following: 1) Grabs a file from a repository, given a file path and revision. 2) Provide a DiffParser subclass that handles pulling out filenames and revisions and any other necessary data from a diff (most of the code for all this is common, so you just hook into things -- see the other files for examples). 3) If Vault has a concept of server-side changesets (you register a changeset with a description, and other data, and the server always knows what you have checked out on the client) then you'll need to implement get_changeset(). So the general model is that this code will have three classes: 1) VaultTool 2) VaultDiffParser 3) VaultClient VaultTool will be a subclass of SCMTool and will be what Review Board talks to. VaultDiffParser will be a subclass of DiffParser and will override functions to parse revision info out of a diff. VaultClient will be a wrapper around the command line tool, which VaultClient will talk to. Now, let's talk diffs. Many revision control systems provide tools that generate diffs unsuitable for Review Board, and sometimes we have to work around them. If vault's tool generates a diff containing revision information for a file that can be used to pull data from the repository, then we're good. If not, you'll need to implement this in post-review. -- -- 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 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: cat: /usr/cc_storage/unix_STREAM/.../myjava.sql@@/main/STREAM/2: No such file or directory\n
Christian,
Thank you for your answer! I'll try to fix it. I think fix is easy,
Michael.
On 13 авг, 00:34, Christian Hammond chip...@chipx86.com wrote:
I think the person who contributed ClearCase support on the server expected
that the repository would be locally checked out with the path given. We'll
certainly take a patch to use cleartool to fix this.
We don't have access to ClearCase, so we can't do a lot of testing
ourselves.
Christian
--
Christian Hammond - chip...@chipx86.com
Review Board -http://www.review-board.org
VMware, Inc. -http://www.vmware.com
On Wed, Aug 12, 2009 at 10:56 AM, MiZhKa miz...@gmail.com wrote:
Hi,
I wonder how reviewboard fetches file versions from ClearCase. Now I
have strange error from /api/json/reviewrequests/74/diff/new/:
{fields: {path: [cat: cat: /usr/cc_storage/unix_STREAM/.../
myjava.sql@@/main/STREAM/2: No such file or directory\n]}, stat:
fail, err: {msg: One or more fields had errors, code: 105}}
It seems that reviewboard (on server side) doesn't use cleartool to
fetch version of file. This can be seen from scmtools/clearcase.py:
class ClearCaseClient:
def __init__(self, path):
self.path = path
def cat_file(self, filename, revision):
p = subprocess.Popen(
['cat', filename],
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
close_fds=(os.name != 'nt')
)
contents = p.stdout.read()
errmsg = p.stderr.read()
failure = p.wait()
if not failure:
return contents
if errmsg.startswith(fatal: Not a valid object name):
raise FileNotFoundError(filename)
else:
raise SCMError(errmsg)
Is it mistake or my misunderstanding?
Thank you for your help!
--~--~-~--~~~---~--~~
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 421 in reviewboard: Enhancement request: nag owners about stale review requests
Updates: Labels: -Milestone-Extensions Milestone-Release1.1 Comment #6 on issue 421 by chipx86: Enhancement request: nag owners about stale review requests http://code.google.com/p/reviewboard/issues/detail?id=421 This actually just needs to be implemented as a management command and run from a cron job. This could make it in before extensions. Let's say 1.1 optimistically, though this may move to 1.2. If someone wants to contribute a patch, this will certainly get in sooner. -- 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 1271 in reviewboard: Need to add copyright headers MIT license text to source files
Status: Accepted Owner: trowbrds Labels: Type-Defect Priority-Medium New issue 1271 by trowbrds: Need to add copyright headers MIT license text to source files http://code.google.com/p/reviewboard/issues/detail?id=1271 For all source files, we need to add copyright headers and the text of the MIT license as a comment at the top. This will make it easier for people to treat it as truly MIT-licensed. -- 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 421 in reviewboard: Enhancement request: nag owners about stale review requests
Comment #7 on issue 421 by jefflamb: Enhancement request: nag owners about stale review requests http://code.google.com/p/reviewboard/issues/detail?id=421 I've created a standalone python script that does this and just uses the API URLs to figure everything out. It currently nags you if you haven't commented on a review request (because it could be the reviewer who hasn't responded to your review's comments.) It also doesn't look at age. If you post a review at 9:59 and the script runs at 10, you'll get pinged. It uses smtplib to send the mail. No idea if this matches everyone's vision for this extension, but what I wrote works for our group. I have a couple of hardcoded variables, our mail server and the RB URL, in the script. Maybe that's ok, though, since it'll just be a cron job. If hardcoded values work for you, it's already done. =) If you need it to be flexible and want to remove the hardcoded stuff, I will not be getting around to it any time soon... -- 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 -~--~~~~--~~--~--~---
