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: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (5dafe99)
Loading...