• 
      

    Switch away from iteration shims.

    Review Request #11850 — Created Oct. 14, 2021 and submitted

    Information

    Review Board
    master

    Reviewers

    In the Python 2 to 3 conversion, many iteration methods (such as
    items() and keys() were changed from returning lists to iterators.
    In order to be correct in usage everywhere, we used six to ensure
    consistent behavior across versions. Now that we're Python 3+ only on
    the master branch, we can get rid of these and just use the builtins.

    Ran unit tests.

    Summary ID
    Switch away from iteration shims.
    In the Python 2 to 3 conversion, many iteration methods (such as `items()` and `keys()` were changed from returning lists to iterators. In order to be correct in usage everywhere, we used `six` to ensure consistent behavior across versions. Now that we're Python 3+ only on the master branch, we can get rid of these and just use the builtins. Testing Done: Ran unit tests.
    e62db00a4e531d577797df057de1f85ab4f67d1a
    Description From Last Updated

    Can we use a keyword argument per line?

    chipx86chipx86

    While here, can we modernize this? all_text_types_extra_data = { _key: {} for _key in extra_text_type_fields.keys() }

    chipx86chipx86

    While here, let's make this a bit nicer/less wasteful and use { ... } instead of set([ ... ]).

    chipx86chipx86

    So here's a question. Why were we even doing this before? Aren't we going from dict to dict? Can't we …

    chipx86chipx86

    I think this can just be list(self.commits.values()).

    chipx86chipx86

    Same here.

    chipx86chipx86

    While here, can we make this a modern dict comprehension?

    chipx86chipx86
    chipx86
    1. 
        
      1. The review on /r/11854/ is relevant to this change as well.

    2. Show all issues

      Can we use a keyword argument per line?

    3. reviewboard/webapi/mixins.py (Diff revision 1)
       
       
       
       
      Show all issues

      While here, can we modernize this?

      all_text_types_extra_data = {
          _key: {}
          for _key in extra_text_type_fields.keys()
      }
      
    4. 
        
    david
    chipx86
    1. 
        
    2. reviewboard/accounts/backends/standard.py (Diff revision 2)
       
       
       
       
       
       
      Show all issues

      While here, let's make this a bit nicer/less wasteful and use { ... } instead of set([ ... ]).

    3. reviewboard/avatars/templatetags/avatars.py (Diff revision 2)
       
       
       
       
       
      Show all issues

      So here's a question. Why were we even doing this before? Aren't we going from dict to dict? Can't we just copy the result of get_avatar_urls()?

      1. I really don't know. Copy/pasteo from inside the avatar services where we have a similar thing but it's calling get_absolute_url, perhaps?

    4. reviewboard/diffviewer/tests/test_commit_utils.py (Diff revision 2)
       
       
       
       
       
      Show all issues

      I think this can just be list(self.commits.values()).

    5. reviewboard/diffviewer/tests/test_commit_utils.py (Diff revision 2)
       
       
       
       
       
      Show all issues

      Same here.

    6. reviewboard/reviews/builtin_fields.py (Diff revision 2)
       
       
       
       
       
      Show all issues

      While here, can we make this a modern dict comprehension?

    7. 
        
    david
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-5.0.x (5dafe99)