mozreview: only mirror full commit description to bugzilla when first publishing (bug 1216078) r=mcote
authorDan Minor <dminor@mozilla.com>
Mon, 19 Oct 2015 13:31:23 -0400
changeset 3206 608df44d9ff308b1f39675f014654e449e0bcff7
parent 3205 f023f0c5f29ca2a568b9d12e61b0e5ca13b2b425
child 3207 fece840df94511da1672e27d897f942672b647d9
push id1202
push userbind-autoland@mozilla.com
push dateFri, 23 Oct 2015 19:05:12 +0000
treeherderversion-control-tools@608df44d9ff3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcote
bugs1216078
mozreview: only mirror full commit description to bugzilla when first publishing (bug 1216078) r=mcote At the moment we create a comment containing the full commit description each time a commit is updated. If people write detailed commit messages, this ends up taking a large amount of space when looking at the bug. With these changes, we only create a comment with the full commit description on the first change. The comment for each subsequent update to a commit only contains a link to the interdiff.
hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
hgext/reviewboard/tests/test-bugzilla-review-interaction.t
hgext/reviewboard/tests/test-bugzilla.t
hgext/reviewboard/tests/test-review-request-closed-discarded.t
pylib/rbbz/rbbz/extension.py
--- a/hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
+++ b/hgext/reviewboard/tests/test-bugzilla-review-flag-cleared.t
@@ -328,17 +328,17 @@ Updating the review request as an L1 aut
       - LGTM
     - author: author@example.com
       id: 6
       tags: []
       text:
       - Comment on attachment 1
       - 'MozReview Request: Bug 1 - Initial commit to review'
       - ''
-      - Bug 1 - Initial commit to review
+      - 'Review request updated; see interdiff: http://*/r/2/diff/1-2/' (glob)
     component: TestComponent
     depends_on: []
     platform: All
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: First Bug
 
@@ -523,17 +523,17 @@ We should have an r+ flag already set.
       - LGTM
     - author: l3author@example.com
       id: 10
       tags: []
       text:
       - Comment on attachment 2
       - 'MozReview Request: Bug 2 - Modified commit to review'
       - ''
-      - Bug 2 - Modified commit to review
+      - 'Review request updated; see interdiff: http://*/r/4/diff/1-2/' (glob)
     component: TestComponent
     depends_on: []
     platform: All
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: Second Bug
 
--- a/hgext/reviewboard/tests/test-bugzilla-review-interaction.t
+++ b/hgext/reviewboard/tests/test-bugzilla-review-interaction.t
@@ -315,17 +315,17 @@ Removing a reviewer should remove their 
       - Bug 2 - Multiple reviewers
     - author: author@example.com
       id: 7
       tags: []
       text:
       - Comment on attachment 2
       - 'MozReview Request: Bug 2 - Multiple reviewers'
       - ''
-      - Bug 2 - Multiple reviewers
+      - 'Review request updated; see interdiff: http://*/r/4/diff/1-2/' (glob)
     component: TestComponent
     depends_on: []
     platform: All
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: Multiple Reviewers
 
@@ -370,25 +370,25 @@ Removing all reviewers should remove all
       - Bug 2 - Multiple reviewers
     - author: author@example.com
       id: 7
       tags: []
       text:
       - Comment on attachment 2
       - 'MozReview Request: Bug 2 - Multiple reviewers'
       - ''
-      - Bug 2 - Multiple reviewers
+      - 'Review request updated; see interdiff: http://*/r/4/diff/1-2/' (glob)
     - author: author@example.com
       id: 8
       tags: []
       text:
       - Comment on attachment 2
       - 'MozReview Request: Bug 2 - Multiple reviewers'
       - ''
-      - Bug 2 - Multiple reviewers
+      - 'Review request updated; see interdiff: http://*/r/4/diff/1-2/' (glob)
     component: TestComponent
     depends_on: []
     platform: All
     product: TestProduct
     resolution: ''
     status: UNCONFIRMED
     summary: Multiple Reviewers
 
--- a/hgext/reviewboard/tests/test-bugzilla.t
+++ b/hgext/reviewboard/tests/test-bugzilla.t
@@ -88,10 +88,80 @@ Publishing the review will add an attach
     component: TestComponent
     depends_on: []
     platform: All
     product: TestProduct
     resolution: ''
     status: NEW
     summary: bug1
 
+We should only display the full commit description the first time it is
+published.
+
+  $ echo foo1 >> foo
+  $ hg commit --amend
+  saved backup bundle to $TESTTMP/client/.hg/strip-backup/a92d53c0ffc7-1dd3de76-amend-backup.hg (glob)
+  $ hg push
+  pushing to ssh://*:$HGPORT6/test-repo (glob)
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
+  remote: recorded push in pushlog
+  submitting 1 changesets for review
+  
+  changeset:  1:ad7618cd44de
+  summary:    Bug 1 - Foo 1
+  review:     http://*:$HGPORT1/r/2 (draft) (glob)
+  
+  review id:  bz://1/mynick
+  review url: http://*:$HGPORT1/r/1 (draft) (glob)
+  (review requests lack reviewers; visit review url to assign reviewers)
+  (visit review url to publish these review requests so others can see them)
+
+  $ rbmanage publish 1
+  $ bugzilla dump-bug 1
+  Bug 1:
+    attachments:
+    - attacher: default@example.com
+      content_type: text/x-review-board-request
+      data: http://*:$HGPORT1/r/2/ (glob)
+      description: 'MozReview Request: Bug 1 - Foo 1'
+      file_name: reviewboard-2-url.txt
+      flags: []
+      id: 1
+      is_obsolete: false
+      is_patch: false
+      summary: 'MozReview Request: Bug 1 - Foo 1'
+    blocks: []
+    cc: []
+    comments:
+    - author: default@example.com
+      id: 1
+      tags: []
+      text: ''
+    - author: default@example.com
+      id: 3
+      tags: []
+      text:
+      - Created attachment 1
+      - 'MozReview Request: Bug 1 - Foo 1'
+      - ''
+      - Bug 1 - Foo 1
+    - author: default@example.com
+      id: 4
+      tags: []
+      text:
+      - Comment on attachment 1
+      - 'MozReview Request: Bug 1 - Foo 1'
+      - ''
+      - 'Review request updated; see interdiff: http://*/r/2/diff/1-2/' (glob)
+    component: TestComponent
+    depends_on: []
+    platform: All
+    product: TestProduct
+    resolution: ''
+    status: NEW
+    summary: bug1
+
   $ mozreview stop
   stopped 10 containers
--- a/hgext/reviewboard/tests/test-review-request-closed-discarded.t
+++ b/hgext/reviewboard/tests/test-review-request-closed-discarded.t
@@ -540,25 +540,25 @@ The attachment for the review request sh
       - Bug 1 - Foo 2
     - author: default@example.com
       id: 4
       tags: []
       text:
       - Comment on attachment 1
       - 'MozReview Request: Bug 1 - Foo 1'
       - ''
-      - Bug 1 - Foo 1
+      - 'Review request updated; see interdiff: http://*/r/2/diff/1-2/' (glob)
     - author: default@example.com
       id: 5
       tags: []
       text:
       - Comment on attachment 2
       - 'MozReview Request: Bug 1 - Foo 2'
       - ''
-      - Bug 1 - Foo 2
+      - 'Review request updated; see interdiff: http://*/r/3/diff/1-2/' (glob)
     component: TestComponent
     depends_on: []
     platform: All
     product: TestProduct
     resolution: ''
     status: NEW
     summary: summary
 
--- a/pylib/rbbz/rbbz/extension.py
+++ b/pylib/rbbz/rbbz/extension.py
@@ -269,24 +269,31 @@ def post_bugzilla_attachment(bugzilla, b
             # The last review given by this reviewer had a ship-it, so we
             # will carry their r+ forward. If someone had manually changed
             # their flag on bugzilla, we may be setting it back to r+, but
             # we will consider the manual flag change on bugzilla user
             # error for now.
             if review.ship_it:
                 reviewers[last_user.email] = True
 
-    comment = review_request_draft.description
+    diffset_count = review_request.diffset_history.diffsets.count()
+    if diffset_count < 1:
+        comment = review_request_draft.description
 
-    if (review_request_draft.changedesc and
-        review_request_draft.changedesc.text):
-        if not comment.endswith('\n'):
-            comment += '\n'
+        if (review_request_draft.changedesc and
+            review_request_draft.changedesc.text):
+            if not comment.endswith('\n'):
+                comment += '\n'
 
-        comment += '\n%s' % review_request_draft.changedesc.text
+            comment += '\n%s' % review_request_draft.changedesc.text
+    else:
+        comment = ('Review request updated; see interdiff: '
+                   '%sdiff/%d-%d/\n' % (get_obj_url(review_request),
+                                        diffset_count,
+                                        diffset_count + 1))
 
     bugzilla.post_rb_url(bug_id,
                          review_request.id,
                          review_request_draft.summary,
                          comment,
                          get_obj_url(review_request),
                          reviewers)