Conversation
|
@austin-owensby Thank you for the contribution! After a quick glance I think the changes you propose make sense, but I need to take a closer look at it and also try to remember why I introduced the I think it makes sense to have a Also, out of interest: what triggered you to look into this, are you using PyGHee for something? |
README.md
Outdated
| To run your GitHub App: | ||
|
|
||
| * Define environment variables for [GitHub Personal Access Token (PAT)](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token) and [GitHub app secret token](https://docs.github.com/en/developers/webhooks-and-events/securing-your-webhooks): | ||
| * Define an environment variables for the [GitHub app secret token](https://docs.github.com/en/developers/webhooks-and-events/securing-your-webhooks): |
There was a problem hiding this comment.
| * Define an environment variables for the [GitHub app secret token](https://docs.github.com/en/developers/webhooks-and-events/securing-your-webhooks): | |
| * Define an environment variable for the [GitHub app secret token](https://docs.github.com/en/developers/webhooks-and-events/securing-your-webhooks): |
pyghee/lib.py
Outdated
| }) | ||
|
|
||
| timestamp = datetime.datetime.utcfromtimestamp(int(event_info['timestamp_raw'])/1000.) | ||
| timestamp = datetime.datetime.fromtimestamp(int(event_info['timestamp_raw'])/1000., datetime.UTC) |
There was a problem hiding this comment.
The datetime documentation recommends to use timezone.utc here, so let's do so?
| timestamp = datetime.datetime.fromtimestamp(int(event_info['timestamp_raw'])/1000., datetime.UTC) | |
| timestamp = datetime.datetime.fromtimestamp(int(event_info['timestamp_raw'])/1000., tz=datetime.UTC) |
There was a problem hiding this comment.
datetime.UTC is an alias for datetime.timezone.utc
https://docs.python.org/3/library/datetime.html#datetime.UTC
There was a problem hiding this comment.
OK, makes sense, but I'm still in favor of adding the tz= to make it explicit we're setting an optional value
Yes I can look into that. At work we were looking for a python library that could handle Webhooks from a GitHub App and make API calls to GitHub using the GitHub app's auth. This library popped up while Googling for something to use. I think ultimately because our client is very stingy about GPL licenses, we won't end up using it, but I figured I'd hop in and make a small update. |
Summary
Remove the unused PyGithub dependency and make additional small updates.
Changes
ghpropertyutcfromtimestampin unit testsImpacts