Re: API usage - getting diffs

2008-10-06 Thread Christian Hammond
On Mon, Oct 6, 2008 at 5:47 PM, Chris Clark <[EMAIL PROTECTED]> wrote:

> On Oct 6, 5:26 pm, "Christian Hammond" <[EMAIL PROTECTED]> wrote:
> > It's possible in that one could modify the code to add this
> functionality.
> > However, I wouldn't add support to show the entire raw diff in the
> upstream
> > codebase, as that would be quite bad for a lot of people (some diffs can
> get
> > very large).
>
> Trying to clarify here, do you mean no diffs at all, or no diffs in
> the template?


I'd rather not modify Review Board to include the diff at all in the e-mail
template or as an attachment. We've had diffs at VMware that have been quite
large (megabytes in size), and I know other places have as well, and this
would certainly put strain on the mail servers.

I guess I wouldn't be too upset with a preference (defaulted to off) that
did include the diff in the template, but I might need more convincing. The
link to the raw diff is right there in the e-mail and should be easy for
anyone to open, but I know that convincing people to use a new product for
something like code review does mean making them feel comfortable using it.

If you want to provide a patch that introduces such a preference and then
support it in the e-mail code and templates, I'll review it.

I know this doesn't help for right now, but down the road a bit (and there's
already a bunch of code for this) we'll be introducing Extension support,
which would allow people to replace or ammend functionality in Review Board.
E-mail being one of these things.

Christian

-- 
Christian Hammond - [EMAIL PROTECTED]
VMware, Inc.

--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~--~~~~--~~--~--~---



Re: API usage - getting diffs

2008-10-06 Thread Chris Clark



On Oct 6, 5:26 pm, "Christian Hammond" <[EMAIL PROTECTED]> wrote:
> It's possible in that one could modify the code to add this functionality.
> However, I wouldn't add support to show the entire raw diff in the upstream
> codebase, as that would be quite bad for a lot of people (some diffs can get
> very large).

Trying to clarify here, do you mean no diffs at all, or no diffs in
the template?

I.e. what would you think about exposing the diffs in
email.mail_review_request(), where "time_emailed" etc. is setup.

Then the end user could decide if they want to keep the default
review_email.txt or modify it to include the raw diffs?

I'm trying to work out how many patches I would need to keep around on
my machine :-) -- not that I've started down this path yet.

As background; I suspect like many people on this list, I'm looking to
persuade the powers-that-be that using ReviewBoard is a good idea. If
I can make ReviewBoard generate the review emails that we currently
generate (almost) by hand with a web interface/post-review I think
I'll have a more compelling case :-)


> What I have considered, and this would be a post-1.0 thing, is to provide
> both text and HTML versions of the e-mail, and in the HTML versions, we'd
> include the diff context just like on the review request page.

That would be very neat!

Chris


--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~--~~~~--~~--~--~---



Re: API usage - getting diffs

2008-10-06 Thread Christian Hammond
It's possible in that one could modify the code to add this functionality.
However, I wouldn't add support to show the entire raw diff in the upstream
codebase, as that would be quite bad for a lot of people (some diffs can get
very large).

What I have considered, and this would be a post-1.0 thing, is to provide
both text and HTML versions of the e-mail, and in the HTML versions, we'd
include the diff context just like on the review request page.

Christian

-- 
Christian Hammond - [EMAIL PROTECTED]
VMware, Inc.


On Mon, Oct 6, 2008 at 5:20 PM, Chris <[EMAIL PROTECTED]> wrote:

>
>
>
> On Oct 6, 12:43 pm, "Christian Hammond" <[EMAIL PROTECTED]> wrote:
> > There's no Web API request for this right now, but you can get it from
> > /r/#/diff/raw/. Or diff/revision_number/raw/ to get a specific revision.
>
> I've a vaguely related question, is there anyway to expose the (raw)
> diffs in the email that is sent out for review?
>
> Chris
>
>
> >
>

--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~--~~~~--~~--~--~---



Re: API usage - getting diffs

2008-10-06 Thread Chris



On Oct 6, 12:43 pm, "Christian Hammond" <[EMAIL PROTECTED]> wrote:
> There's no Web API request for this right now, but you can get it from
> /r/#/diff/raw/. Or diff/revision_number/raw/ to get a specific revision.

I've a vaguely related question, is there anyway to expose the (raw)
diffs in the email that is sent out for review?

Chris


--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~--~~~~--~~--~--~---



Re: API usage - getting diffs

2008-10-06 Thread Christian Hammond
There's no Web API request for this right now, but you can get it from
/r/#/diff/raw/. Or diff/revision_number/raw/ to get a specific revision.

Christian

-- 
Christian Hammond - [EMAIL PROTECTED]
VMware, Inc.


On Mon, Oct 6, 2008 at 12:26 PM, yop <[EMAIL PROTECTED]> wrote:

>
> I think that the title sums it up. I force a 404 error on the demo
> review-board to get the list if the available API requests but I just
> can't seem to short out how to get the diff for a specific review.
> http://code.google.com/p/reviewboard/wiki/ReviewBoardAPI isn't that
> helpful ;-)
>
> Regards,
> Yorgos
> >
>

--~--~-~--~~~---~--~~
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 [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~--~~~~--~~--~--~---