Deprecate the type argument of review request signals

Review Request #8896 — Created April 12, 2017 and submitted

Information

Review Board
release-3.0.x

Reviewers

This patch deprecates the type argument of the review_request_closed
and review_request_closing signals, due to shadowing the type
builtin. We still provide the type argument, but it is wrapped in a
lazy object that will emit a warning when casted to a unicode string. We
now include the close_type argument which takes the place of the old
type argument. Usages and unit tests have been updated for this new
behaviour.

Ran unit tests.

Description From Last Updated

Typo in summary: rquest

daviddavid

this file isnt supposed to be in the diff.

brenniebrennie

Sooo the difficulty here is that if an extension does: review_request.close(type=..., ...) then things are going to break. The problem …

chipx86chipx86

Can you pass these as keyword arguments? It'll help with readability a lot.

chipx86chipx86

Here, too.

chipx86chipx86

This should have a .. deprecated::

chipx86chipx86

Here, too.

chipx86chipx86

This would be great in Djblets :)

chipx86chipx86
brennie
brennie
  1. 
      
  2. Show all issues

    this file isnt supposed to be in the diff.

  3. 
      
brennie
david
  1. 
      
  2. Show all issues

    Typo in summary: rquest

  3. 
      
brennie
chipx86
  1. 
      
  2. Show all issues

    Sooo the difficulty here is that if an extension does:

    review_request.close(type=..., ...)
    

    then things are going to break. The problem is that if **kwargs is provided, and we try to check for it there, then it'll still break since close_type won't be supported.

    Hmm..

    Not the end of the world, but would be nice to fix.

    Perhaps we could say that close() will, by default, close as submitted. Then we can do that and have a **kwargs that checks for type and overrides close_type?

    I think submitted is going to be far more common than discarded, so I wouldn't have a problem with this as a default.

  3. Show all issues

    Can you pass these as keyword arguments? It'll help with readability a lot.

  4. reviewboard/reviews/models/review_request.py (Diff revision 3)
     
     
     
     
    Show all issues

    Here, too.

  5. reviewboard/reviews/signals.py (Diff revision 3)
     
     
     
     
    Show all issues

    This should have a .. deprecated::

  6. reviewboard/reviews/signals.py (Diff revision 3)
     
     
     
    Show all issues

    Here, too.

  7. reviewboard/signals.py (Diff revision 3)
     
     
    Show all issues

    This would be great in Djblets :)

    1. Can we put this into another change later? I really wanna get this in.

  8. 
      
brennie
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (9a02c1e)