Factor out common parts of ReviewRequest and ReviewRequestDraft.

Review Request #3265 — Created Aug. 9, 2012 and submitted

Information

Review Board

Reviewers

Factor out common parts of ReviewRequest and ReviewRequestDraft.

ReviewRequest and ReviewRequestDraft have always had many fields and
methods that operated the same way but were defined separately. This has
led to a lot of duplicate code, which has always been a pain to keep
updated.

Now these common fields and methods have been split out into a new
base class, BaseReviewRequestDetails. This covers the summary,
description, testing done, bugs closed, branch, field normalization
during save, human-readable bug list, screenshot/file attachment
accessors, default reviewers, and updating from a changenum.
General playing around with the review requests seems to work.

All unit tests pass.
Description From Last Updated

Why not use bugs.sort(key=int)?

ME medanat
ME
  1. 
      
  2. reviewboard/reviews/models.py (Diff revision 1)
     
     
    Why not use bugs.sort(key=int)?
    1. Good question. Old code :) Will update.
  3. 
      
chipx86
ME
  1. 
      
  2. reviewboard/reviews/models.py (Diff revision 2)
     
     
    Is it possible to have numbers of type <int> in the bugs list?
    
    ie: [1, 2], rather than: ["1", "2"]
    
    If not, why not use bugs.sort(key=str.lower) here to make it case insensitive.
    
    Just a suggestion.
  3. reviewboard/reviews/models.py (Diff revision 2)
     
     
    Is it possible to have numbers of type <int> in the bugs list?
    
    ie: [1, 2], rather than: ["1", "2"]
    
    If not, why not use bugs.sort(key=str.lower) here to make it case insensitive.
    
    Just a suggestion.
    1. Double post bug?
    2. Weird.
      
      If the bug tracker uses ints, we absolutely want to cast (otherwise, "12" < "9"). Otherwise, they're alpha-numeric, and I just don't want to assume that "A" is equal to "a" in that case.
      
      They're stored in the database as a comma-separated string of numbers, so when we parse that out, we get strings back. They'll never start off as an int.
      
      This is just moved logic. The last thing I want to do right now is change how it works and possibly break somebody.
  4. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (7895266)
Loading...