811: Discarding an update diff posts data it shouldn't

jeff****@gmai***** (Google Code) (Is this you? Claim this profile.)
chipx86
 chipx86
Sept. 23, 2009 
What steps will reproduce the problem?
1. Post a review.  Publish.
2. Post a diff.  Discard the diff.

What is the expected output? What do you see instead?

A "Review request changed" box with blank content is posted to the end of 
the review.  As a side-effect, all reviewers get emailed on-discard as 
well.

Using RB r1668.
chipx86
#1 chipx86 
I'm confused about the repro case. This extra review with blank content is in
addition to the first review posted? I can't seem to get this to repro.
  • +NeedInfo
#2 jeff****@gmai***** (Google Code) (Is this you? Claim this profile.) 
Ok, this is exactly how I reproduce it.  Check something out, make a change.
Run post-review like so:
post-review.py . --target-people=person1 -o
It opens the page in a browser.  I fill in description, testing, summary.  Hit 
publish.

Change one line in another file in the . directory.
Send off an update diff:
post-review.py . --target-people=person1 -o -r 123

I have "This review request is a draft." in a big green box at top with 
publish/discard changes buttons.  At the bottom of my review, I have a "Review 
request changed" box, even though I didn't publish the new diff.  The content of this 
box is blank.  This also kicks off an update email to the reviewers.

If I publish the changes, I correctly get another box after the empty "Review request 
changed" box that says:
Review request changed
Diff
  added Diff r2
#3 jeff****@gmai***** (Google Code) (Is this you? Claim this profile.) 
Figured it out.

Specifying "--target-people=person1,person2" and "-r 123" where review #123 already 
has person1 and person2 in the reviewer's "people" list creates this blank update.  
Whether or not you want to "fix" this, I leave up to you.
david
#4 david
  • -NeedInfo
    +New
chipx86
#5 chipx86 
Turned out this was a bug in post-review. We were publishing the draft always on
setting a field, and were doing so before uploading the diff even. In this case,
nothing actually changed, hence the empty change description.

Fixed in rbtools rc5ce084.
  • -New
    +Fixed
  • +Component-RBTools
    +Milestone-RBTools-Release1.0
  • +chipx86