Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Honeycomb support #213

Merged
merged 9 commits into from Jun 18, 2021
Merged

Honeycomb support #213

merged 9 commits into from Jun 18, 2021

Conversation

TheLonelyGhost
Copy link
Member

No description provided.

@TheLonelyGhost TheLonelyGhost requested a review from a team as a code owner June 6, 2021 07:00
@MaxVanDeursen
Copy link
Member

Looks sweet!

MaxVanDeursen
MaxVanDeursen previously approved these changes Jun 7, 2021
@TheLonelyGhost
Copy link
Member Author

Some fun with getting the annotations in tor/core/config.py to actually play nicely with mypy, but once I figure that out it should have green checkmarks all the way.

@itsthejoker
Copy link
Member

What do we need to do for deploying this?

@TheLonelyGhost
Copy link
Member Author

TheLonelyGhost commented Jun 8, 2021

Needs some TLC to make mypy not fail spectacularly on upgrading to python 3.9.

I don't want to rip out the types again, but I'm not yet sure what the problem is that it has with them.

IIRC it's a runtime issue too, not just static analysis.

@TheLonelyGhost
Copy link
Member Author

Okay, looks like it may just require updating mypy

python/mypy#9761

@TheLonelyGhost TheLonelyGhost requested review from MaxVanDeursen and a team and removed request for MaxVanDeursen June 12, 2021 04:35
@TheLonelyGhost
Copy link
Member Author

Upgrading mypy to >= v0.800 is what fixed the python 3.9 support. Now we're on v0.902 of it, so I hope python 3.10 doesn't introduce any unexpected problems like the last "minor" upgrade.

- prawcore (2.0.0 -> 2.2.0)
- websocket-client (1.0.1 -> 0.54.0)
- slackclient (1.0.6 -> 1.3.2)
@@ -1,13 +1,14 @@
from datetime import datetime, timedelta

from praw import Reddit
from praw.exceptions import RedditAPIException
# from praw.exceptions import RedditAPIException
Copy link
Member

Choose a reason for hiding this comment

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

Better not to comment code

Copy link
Member

Choose a reason for hiding this comment

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

This is a throwaway script that's supposed to be modified in place depending on whatever we need -- if this is the only issue you have with the PR, then I would consider it good to go

Copy link
Member Author

Choose a reason for hiding this comment

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

^^^ This.

Main reason I commented it out here instead of deleting is because some seemingly important try-except code was commented out later in the file, where it handles this particular exception. Rather than delete the commented code in its entirety, this file seems more like a living workspace just in case we need it. Might even be able to delete it if we rework it into one of the commands in the bot. 🤷

@TheLonelyGhost TheLonelyGhost removed the request for review from MaxVanDeursen June 18, 2021 15:47
@itsthejoker itsthejoker dismissed MaxVanDeursen’s stale review June 18, 2021 15:47

Changes requested in file outside of scope

@itsthejoker itsthejoker merged commit fcd53af into main Jun 18, 2021
@itsthejoker itsthejoker deleted the honeycomb branch June 18, 2021 15:47
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.

None yet

3 participants