2106: Diff updates after "ship it"

jonathan.mc************@gmai***** (Google Code) (Is this you? Claim this profile.)
3925 
What version are you running?
V1.5.5

What's the URL of the page this enhancement relates to, if any?
/dashboard/

Describe the enhancement and the motivation for it.
Enhancement: Clear a review's "ship it" status in the dashboard when a new diff is added to a review after a reviewer has selected "ship it". 
Motivation: In our day to day use of Reviewboard our reviews go back & forth for quite a while and it is very common for one reviewer to say "ship it" and another to identify a problem afterwards. Revisions are made to the software and the diff updated (often many times) meaning that the "ship it" status on the dashboard very rarely indicates the actual current status (or acceptability) of the review.

What operating system are you using? What browser?
Win7, chrome

Please provide any additional information below.
N/A
#1 intr****@gmai***** (Google Code) (Is this you? Claim this profile.) 
Same issue for our project.
chipx86
#2 chipx86 
We'll likely revisit how Ship It works in the next release. It certainly needs some changes made.
  • +Confirmed
  • +Milestone-Release1.7
    +Component-Reviews
david
#3 david
  • -Milestone-Release1.7
    +Milestone-Release1.8
david
#4 david
  • -Milestone-Release1.8
david
#5 david
#6 tjohnson2 
Adding my name to the list of people/companies who would love to see this functionality.
misery
#7 misery 
I added a litte patch as a work-around.

https://reviews.reviewboard.org/r/7825/
  • +
    0001-Show-another-column-color-if-a-review-request-was-ch.patch 
    From eb314e26f3d79b39aa22164b209921b39421048b Mon Sep 17 00:00:00 2001
    From: =?UTF-8?q?Andr=C3=A9=20Klitzing?= <aklitzing@gmail.com>
    Date: Wed, 23 Dec 2015 11:20:36 +0100
    Subject: [PATCH] Show another column color if a review request was changed
     after a ShipIt
    If someone edit the review request it cannot be identified in overview
    page that the ShipIt means only old review requests. So we show
    the ShipIt column in another color if there is no ShipIt for the
    latest review request.
    ---
     reviewboard/datagrids/columns.py            | 19 +++++++++++++++++--
     reviewboard/static/rb/css/defs.less         |  2 ++
     reviewboard/static/rb/css/ui/datagrids.less |  9 +++++++++
     3 files changed, 28 insertions(+), 2 deletions(-)
    diff --git x/reviewboard/datagrids/columns.py y/reviewboard/datagrids/columns.py
    index 0ad3c45..fca0f2f 100644
    --- x/reviewboard/datagrids/columns.py
    +++ y/reviewboard/datagrids/columns.py
    @@ -573,11 +573,26 @@ class ShipItColumn(Column):
                         '</span>'
         
    
 
    

    


    
   
    • 


  


 








 

  

   

  

 
 

  

    
     

   
   #8
   xelepart


  


  
Adding my company to the list. This is definitely how we use ship its, and find the "ship it count" very confusing (but still critical... just really annoying 'cause it's both critical and wrong :D )


Would love to see this fixed. Five years after the original request.



 








 

  

   Misery

  

 
 

  

    
     

   
   #9
   Misery


  


  
I added our stuff to an extension that we use internally.


Maybe someone will use this, too.

https://github.com/misery/ExtendedApproval


Use "pip wheel ." to get an installable whl file.


This will add an additional column "Approval" and set the approval flag that can be checked with the WebAPI.


This is a really simple extension. Maybe we will add some configuration to it. Or someone wants to contribute. :-)


If you use Mercurial... you could use our hook, too.

https://reviews.reviewboard.org/r/8554/



 








 

  

   Misery

  

 
 

  

    
     

   
   #10
   Misery


  


  
Since last commit (https://github.com/reviewboard/reviewboard/commit/260637db51adf475d16a437cc9a38aa15d6b4720) the ShipIt column was improved for "issue-verifying".


There is an explicit "approved" flag of reviewboard but there is no indicator for this.

Every extension or API user should use "approved" to check if a review request is "approved".

But reviewboard itself uses the incorrect "ShipIt" count directly. Why not use another color for the shipit-count if "approved" is false? Like I did in a previous patch. I don't mean to use a calculation for another ShipIt count. Just to indicate the "approved" flag of reviewboard API.


Example:

https://reviews.reviewboard.org/r/7825/file/1156/



 








 

  

   Misery

  

 
 

  

    
     

   
   #11
   Misery


  


  
Since 3.0 it is possible to revoke ShipIts. I added this to my extension. After someone posts an update it will revoke any previous ShipIt.


https://github.com/misery/ExtendedApproval



 








 

  

   chipx86

  

 
 

  

    
     

   
   #12
   chipx86


  


  
Pushed to release-5.0.x (ce79281706321b36ca499e1332a769e191e20574).



  
    


   
  • 
    
    
    -Confirmed
    +Fixed
    
   
    •