Re: Problem with uploading consecutive diffs with updates

2012-05-15 Thread Robert Dailey
This happens outside of interdiff too. There is never a reason to say
something can't be fixed. It may be difficult, but not impossible.

Either review-board or post-review has access to file revisions in
Perforce. All you need to do is simply set your base (original) in the diff
to be locked to the latest revision of the file (or at least match the base
revision that is in my changelist). post-review can check this prior to
updating an existing review, so there's no reason this can't be done. I
don't know the logistics of how Review Board or post-review works, but in
theory this is a solvable problem.

On Mon, May 14, 2012 at 8:12 PM, Christian Hammond chip...@chipx86.comwrote:

 Oh! The interdiff was the key thing.

 So, yes, viewing a diff between uploaded diffs will give you that result.
 There's really not anything we can do about this. What that is really doing
 is taking the patched set of files in r1 and diffing them against the
 patched set in r2. Since there are changes in r2 that came about from the
 sync, you're going to get some junk in there.

 It's annoying, but we basically don't know any better. There's a bug open
 about this, but it'll probably stay open forever.

 One option that some people use (it's a bit manual, but...) is to sync
 with the same changes in r1 and post that up, and *then* put the changes
 from r2 up. However, this is kind of time-consuming and annoying without
 some patch management system inbetween (like quilt or Git).


 Christian

 --
 Christian Hammond - chip...@chipx86.com
 Review Board - http://www.reviewboard.org
 VMware, Inc. - http://www.vmware.com


 On Mon, May 14, 2012 at 10:28 AM, Robert Dailey 
 rcdailey.li...@gmail.comwrote:

 When I say get latest I'm referring to the Get Latest Revision option
 in P4V. I don't use p4 directly, so I'm not sure what command it maps to.

 Right click in your workspace view on the folder you want to get latest
 changes, and click Get Latest Revision. This will pull down the latest
 changes in perforce to files.

 Your description is almost right. I've corrected it below:

 You have a changelist with some code in it. You sync, new changes are
 pulled in. You *re-*post the *existing* changeset to Review Board, and
 the diff is including the things that came from the sync?

 This is absolutely correct. Note that I'm only working with 1 changelist.
 I update my existing review with the same changelist a second time, and any
 updates pulled from the perforce server (that other people made) will
 appear in the diff on Review Board. Also please note that all I'm doing is
 running the post-review command twice:

 1) Create a new changelist, modify some source code
 2) Use that post-review command to create a new review on Review Board.
 3) Run Get Latest Revision in P4V to pull down latest source code
 changes. Files previously put up for review are modified by the sync. I
 resolve any conflicts and make more changes to the source files.
 4) Run the same post-review command a second time, same one as before,
 same changelist number
 5) View the diff online at Review Board server, and do a diff between r1
 and r2, in this diff I see the changes I made in r2 as well as the diff for
 code changes pulled down from the Perforce server (those are the changes I
 did not make).

 Hope this makes more sense.

 Is this a problem with post-review, or am I simply not running the right
 commands?


 On Sun, May 13, 2012 at 5:48 PM, Christian Hammond 
 chip...@chipx86.comwrote:

 Hi Robert,

 Sorry for the late reply. I've been on vacation.

 I'm not sure I'm fully understanding the problem you're hitting. Let me
 see if I have this right. You have a changelist with some code in it. You
 sync, new changes are pulled in. You post the new changeset to Review
 Board, and the diff is including the things that came from the sync?

 I've never seen this behavior, and use Review Board with Perforce almost
 daily. Posting a new review request *should* include only what changed for
 those files.

 When you say a get latest, is this 'p4 sync', or something else?

 This sounds like this is a client-side problem, where the changes aren't
 resolved properly or the metadata isn't being updated properly, causing the
 revision for the files to be incorrect.

 Christian

 --
 Christian Hammond - chip...@chipx86.com
 Review Board - http://www.reviewboard.org
 VMware, Inc. - http://www.vmware.com


 On Mon, May 7, 2012 at 8:39 AM, Robert Dailey 
 rcdailey.li...@gmail.comwrote:

 Can someone help me out here?


 On Wed, May 2, 2012 at 1:37 PM, Robert Dailey rcdailey.li...@gmail.com
  wrote:

 So I'm using Perforce and I upload my initial reviews (as well as
 follow up changes and updates) using this command. Note that this is a
 Custom Tool in P4V:

 %C --server=http://reviewboard.company.com --p4-client=$c
 --p4-port=$p --username=user --password=password

 %C is the changelist number in Perforce
 $c is the current workspace
 $p is the current port

 The problem 

Re: Problem with uploading consecutive diffs with updates

2012-05-15 Thread Christian Hammond
I think there's a misunderstanding of the problem.

If viewing a single diff (not comparing against two) is showing extra stuff
in your change, then something is going wrong in your Perforce client,
causing it to report improper revisions when building the diff. I can't say
what that is, because it's something I've never heard of happening before
(and perforce is very heavily used with Review Board). We're dependent on
that revision information being correct, since that's what we fetch from
the Perforce server when rendering the diff. We have no control over that
information.

The interdiffs are a whole different matter. An interdiff is a comparison
between two sets of patched files, so we're dependent on whatever was
uploaded. If that's a patch on revision 5 and another on revision 10, and
changes have been made to that file between those revisions, we're going to
get those changes because that's what a diff between those revisions will
show. We don't know any better.

We can't just sync to one revision and apply both patches, because a good
amount of the time, that just doesn't work. This is no different of a
problem than a developer's standard sync/resolve cycle. Imagine if that can
be automated 100% of the time, correctly. It can't, really, because the
correct result is not mathematical but rather dependent on what the user
intends as the correctly resolved set of changes. We'd need to get this
right, somehow, to show these differences properly.

There are potentially things that can be done to better weed out parts of
the diff for unrelated chunks of code, but that's a pretty hard diff
analyses problem and I don't know that anyone's solved it. Certainly, we
have not, and I haven't seen solutions out there. Even if you get those
right, you're still going to inevitably get things wrong within the
vicinity of code you do want to see. And you may even lose that code, which
defeats the entire purpose.

Believe me, we've thought about this a lot and right now, unless someone
comes up with a model that hasn't been explored,  I'm saying that given
what information Review Board knows and what we can do and the algorithms
available, this can't be fixed. Maybe that will change someday, but today
that's my answer.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Tue, May 15, 2012 at 8:46 AM, Robert Dailey rcdailey.li...@gmail.comwrote:

 This happens outside of interdiff too. There is never a reason to say
 something can't be fixed. It may be difficult, but not impossible.

 Either review-board or post-review has access to file revisions in
 Perforce. All you need to do is simply set your base (original) in the diff
 to be locked to the latest revision of the file (or at least match the base
 revision that is in my changelist). post-review can check this prior to
 updating an existing review, so there's no reason this can't be done. I
 don't know the logistics of how Review Board or post-review works, but in
 theory this is a solvable problem.


 On Mon, May 14, 2012 at 8:12 PM, Christian Hammond chip...@chipx86.comwrote:

 Oh! The interdiff was the key thing.

 So, yes, viewing a diff between uploaded diffs will give you that result.
 There's really not anything we can do about this. What that is really doing
 is taking the patched set of files in r1 and diffing them against the
 patched set in r2. Since there are changes in r2 that came about from the
 sync, you're going to get some junk in there.

 It's annoying, but we basically don't know any better. There's a bug open
 about this, but it'll probably stay open forever.

 One option that some people use (it's a bit manual, but...) is to sync
 with the same changes in r1 and post that up, and *then* put the changes
 from r2 up. However, this is kind of time-consuming and annoying without
 some patch management system inbetween (like quilt or Git).


 Christian

 --
 Christian Hammond - chip...@chipx86.com
 Review Board - http://www.reviewboard.org
 VMware, Inc. - http://www.vmware.com


 On Mon, May 14, 2012 at 10:28 AM, Robert Dailey rcdailey.li...@gmail.com
  wrote:

 When I say get latest I'm referring to the Get Latest Revision
 option in P4V. I don't use p4 directly, so I'm not sure what command it
 maps to.

 Right click in your workspace view on the folder you want to get latest
 changes, and click Get Latest Revision. This will pull down the latest
 changes in perforce to files.

 Your description is almost right. I've corrected it below:

 You have a changelist with some code in it. You sync, new changes are
 pulled in. You *re-*post the *existing* changeset to Review Board, and
 the diff is including the things that came from the sync?

 This is absolutely correct. Note that I'm only working with 1
 changelist. I update my existing review with the same changelist a second
 time, and any updates pulled from the perforce server (that other people
 made) will 

Re: post-review throw diff error

2012-05-15 Thread Nilesh Jaiswal
Hi Chris,

Could you please provide me a solution to resolve the issue.

Regards,
Nilesh

On Mon, May 14, 2012 at 3:40 PM, Nilesh Jaiswal nileshj...@gmail.comwrote:

 is this patch version mentioned in the email you are talking about.? Or
 something else.

 What do i need to do?


 On Mon, May 14, 2012 at 10:48 AM, Nilesh Jaiswal nileshj...@gmail.comwrote:

 If you are asking about the version path util then here it is.

 patch -version
 patch 2.5.4
 Copyright 1984-1988 Larry Wall
 Copyright 1989-1999 Free Software Foundation, Inc.

 Please correct me if i am wrong.


 On Mon, May 14, 2012 at 10:43 AM, Nilesh Jaiswal nileshj...@gmail.comwrote:

 How to get the version of GNU patch. Are you asking about the version of
 post-review util.?


 On Mon, May 14, 2012 at 4:08 AM, Christian Hammond 
 chip...@chipx86.comwrote:

 Hi Nilesh,

 We run everything through GNU patch. That generally will handle the \
 No newline at end of file.

 What version of GNU patch is running on that server? What happens if
 you try to apply that same patch on a developer machine?

 Christian

 --
 Christian Hammond - chip...@chipx86.com
 Review Board - http://www.reviewboard.org
 VMware, Inc. - http://www.vmware.com


 On Thu, May 10, 2012 at 12:06 AM, Nilesh Jaiswal 
 nileshj...@gmail.comwrote:

 Dear All,

 I have posted the queries on this but i haven't got any answer hence i
 am raising this queries again.

 After posting review request and then clicking on viewdiff link on RB
 server i get following error message for the files.

 The patch to
 '//sdm/DEV/V14_2_App/XSDGFDH/portals/sidadfds/resources/ApplicationManager.js'
 didn't apply cleanly. The temporary files have been left in
 '/tmp/reviewboard.pgHnuQ' for debugging purposes. `patch` returned:
 patching file /tmp/reviewboard.pgHnuQ/tmpVIuwIQ Hunk #2 FAILED at 931. 1
 out of 2 hunks FAILED -- saving rejects to file
 /tmp/reviewboard.pgHnuQ/tmpVIuwIQ-new.rej

 After looking into the /tmp/reviewboard.pgHnuQ/tmpVIuwIQ-new.rejor
 diff file in /tmp of RB server its says

 Pasting diff snippet which contains the following string (*\ No
 newline at end of file*) in ActionStatusEventHandlerThread.java.diff
 which has caused the diff error. why can we not handle such cases in
 viewdiff.py script. Do we have any patch for the same. I would appreciate
 if you could provide me solution that will solve 100's of such failure
 issues.





 private void log(Exception e) {

 @@ -86,4 +93,4 @@ public class ActionStatusEventHandlerThr

 return ActionStatusEventHandlerThread;

 }



 -}

 +}

 *\ No newline at end of file*

 Regards,
 Nilesh J

  --
 Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
 Happy user? Let us know at http://www.reviewboard.org/users/
 -~--~~~~--~~--~--~---
 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


  --
 Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
 Happy user? Let us know at http://www.reviewboard.org/users/
 -~--~~~~--~~--~--~---
 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






-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
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: Review Request not published

2012-05-15 Thread Christian Hammond
Hi,

It shouldn't be confused under those cases.

If you have Chrome or a modern Firefox, it would really help to see the
network transfer information on that request. That is, the HTTP request and
response headers and payload. Both should offer a tab in their script
developer UI for showing network transfers, and you should be able to dig
in and see exactly what was sent and received.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Tue, May 15, 2012 at 9:41 PM, Geert Linders glind...@dynamiccontrols.com
 wrote:

 Hi Christian,

 I have the same problem when I try to publish or discard a review on a
 particular review. I'm using RB 1.6.4.1. The server logs don't show
 anything when the fault happens (I've set it to DEBUG level). That makes
 sense because the response is 200 OK, which means success in HTTP world.

 I added two screen shots. The first rb_flt1.png shows the response after I
 click on Publish or Discard. The second rb_flt2.png shows the error
 detail.

 I did the same on another review: That worked fine, it saved my changes
 without complaining. It seems as if this particular review is corrupt
 somewhere. Could RB become confused if two people are reviewing at the same
 time i.e. both have a pending review?

 I can't reset the server now because it's running in a production
 environment and is constantly being used.

 Cheers - Geert

  --
 Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
 Happy user? Let us know at http://www.reviewboard.org/users/
 -~--~~~~--~~--~--~---
 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


-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
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: post-review throw diff error

2012-05-15 Thread Christian Hammond
Hi Nilesh,

That version seems right, but I don't know why you're hitting that problem.
Was that the version of patch running on the server?

What happens if you try patching that file locally with the same patch?

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Tue, May 15, 2012 at 9:52 PM, Nilesh Jaiswal nileshj...@gmail.comwrote:

 Hi Chris,

 Could you please provide me a solution to resolve the issue.

 Regards,
 Nilesh


 On Mon, May 14, 2012 at 3:40 PM, Nilesh Jaiswal nileshj...@gmail.comwrote:

 is this patch version mentioned in the email you are talking about.? Or
 something else.

 What do i need to do?


 On Mon, May 14, 2012 at 10:48 AM, Nilesh Jaiswal nileshj...@gmail.comwrote:

 If you are asking about the version path util then here it is.

 patch -version
 patch 2.5.4
 Copyright 1984-1988 Larry Wall
 Copyright 1989-1999 Free Software Foundation, Inc.

 Please correct me if i am wrong.


 On Mon, May 14, 2012 at 10:43 AM, Nilesh Jaiswal 
 nileshj...@gmail.comwrote:

 How to get the version of GNU patch. Are you asking about the version
 of post-review util.?


 On Mon, May 14, 2012 at 4:08 AM, Christian Hammond chip...@chipx86.com
  wrote:

 Hi Nilesh,

 We run everything through GNU patch. That generally will handle the \
 No newline at end of file.

 What version of GNU patch is running on that server? What happens if
 you try to apply that same patch on a developer machine?

 Christian

 --
 Christian Hammond - chip...@chipx86.com
 Review Board - http://www.reviewboard.org
 VMware, Inc. - http://www.vmware.com


 On Thu, May 10, 2012 at 12:06 AM, Nilesh Jaiswal nileshj...@gmail.com
  wrote:

 Dear All,

 I have posted the queries on this but i haven't got any answer hence
 i am raising this queries again.

 After posting review request and then clicking on viewdiff link on RB
 server i get following error message for the files.

 The patch to
 '//sdm/DEV/V14_2_App/XSDGFDH/portals/sidadfds/resources/ApplicationManager.js'
 didn't apply cleanly. The temporary files have been left in
 '/tmp/reviewboard.pgHnuQ' for debugging purposes. `patch` returned:
 patching file /tmp/reviewboard.pgHnuQ/tmpVIuwIQ Hunk #2 FAILED at 931. 1
 out of 2 hunks FAILED -- saving rejects to file
 /tmp/reviewboard.pgHnuQ/tmpVIuwIQ-new.rej

 After looking into the /tmp/reviewboard.pgHnuQ/tmpVIuwIQ-new.rejor
 diff file in /tmp of RB server its says

 Pasting diff snippet which contains the following string (*\ No
 newline at end of file*) in ActionStatusEventHandlerThread.java.diff
 which has caused the diff error. why can we not handle such cases in
 viewdiff.py script. Do we have any patch for the same. I would appreciate
 if you could provide me solution that will solve 100's of such failure
 issues.





 private void log(Exception e) {

 @@ -86,4 +93,4 @@ public class ActionStatusEventHandlerThr

 return ActionStatusEventHandlerThread;

 }



 -}

 +}

 *\ No newline at end of file*

 Regards,
 Nilesh J

  --
 Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
 Happy user? Let us know at http://www.reviewboard.org/users/
 -~--~~~~--~~--~--~---
 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


  --
 Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
 Happy user? Let us know at http://www.reviewboard.org/users/
 -~--~~~~--~~--~--~---
 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





  --
 Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
 Happy user? Let us know at http://www.reviewboard.org/users/
 -~--~~~~--~~--~--~---
 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


-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~--~~~~--~~--~--~---
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: Issue 2585 in reviewboard: Reviewboard's Mercurial Support won't work against Google Code

2012-05-15 Thread reviewboard


Comment #1 on issue 2585 by nate.sku...@gmail.com: Reviewboard's Mercurial  
Support won't work against Google Code

http://code.google.com/p/reviewboard/issues/detail?id=2585

This patch against 1.6.6 fixes it for us.

Attachments:
reviewboard-googlecode-patch.patch  920 bytes

--
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.



Re: Issue 2585 in reviewboard: Reviewboard's Mercurial Support won't work against Google Code

2012-05-15 Thread reviewboard

Updates:
Status: PendingReview
Labels: Milestone-Release1.6.x Component-SCMTools

Comment #2 on issue 2585 by chip...@gmail.com: Reviewboard's Mercurial  
Support won't work against Google Code

http://code.google.com/p/reviewboard/issues/detail?id=2585

Can you post this on http://reviews.reviewboard.org/ ?

--
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.



Re: Issue 1664 in reviewboard: Chinese characters in code cannot be shown correctly

2012-05-15 Thread reviewboard


Comment #6 on issue 1664 by csfreeb...@gmail.com: Chinese characters in  
code cannot be shown correctly

http://code.google.com/p/reviewboard/issues/detail?id=1664

“after set the repository's encoding as gb2312”,I don't know how to do it.  
I am using git with review board 1.6.6.


--
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.