Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Kill off some old python 2 code #7519

Merged
merged 6 commits into from
May 18, 2020
Merged

Kill off some old python 2 code #7519

merged 6 commits into from
May 18, 2020

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented May 15, 2020

This stuff has been annoying me for a while.

Each commit should stand alone.

(There's plenty more that can be removed; this was just what I found in 10 minutes.)

richvdh added 2 commits May 15, 2020 19:07
this is a no-op on python 3.
this is a no-op on python 3.
@richvdh richvdh requested a review from a team May 15, 2020 18:34
@richvdh richvdh force-pushed the rav/kill_py2_code branch from b8c823b to ab57353 Compare May 15, 2020 18:37
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice clean-up!

@@ -72,7 +67,7 @@ def rows_v2(server, json):
valid_until = json["valid_until_ts"]
key_json = encode_canonical_json(json)
for key_id in json["verify_keys"]:
yield (server, key_id, "-", valid_until, valid_until, db_type(key_json))
yield (server, key_id, "-", valid_until, valid_until, db_binary_type(key_json))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why you didn't just use memoryview directly here (and some other places)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda felt like maybe the abstraction was useful. I went back and forth on it a bit 🤷‍♂️

@richvdh richvdh merged commit 4d1afb1 into develop May 18, 2020
@richvdh richvdh deleted the rav/kill_py2_code branch May 18, 2020 09:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants