Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove time of day and tz info properly #83

Merged
merged 4 commits into from
Apr 2, 2024
Merged

fix: remove time of day and tz info properly #83

merged 4 commits into from
Apr 2, 2024

Conversation

zkoppert
Copy link
Member

@zkoppert zkoppert commented Apr 2, 2024

Pull Request

Proposed Changes

fixes #82

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing

Reviewer

  • Label as either bug, documentation, enhancement, infrastructure, or breaking

@zkoppert zkoppert requested a review from jmeridth as a code owner April 2, 2024 00:24
@zkoppert zkoppert added the bug Something isn't working label Apr 2, 2024
@zkoppert
Copy link
Member Author

zkoppert commented Apr 2, 2024

@jmeridth Can add a test here if you are up for it and merge them together.

Copy link
Member

@jmeridth jmeridth left a comment

Choose a reason for hiding this comment

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

LGTM. If we need to deploy now I'm good. Can add test later tonight when back at keyboard.

- [x] change from splitting string to using datetime.datetime.fromisoformat
  since the format from GitHub API for a repositories created_at date
  is ISO 8601 (example: 2024-04-03T05:00:00Z)
- [x] write tests to handle if the repo.created_at is a string or a datetime

According to the github3.py library, the repository's [created_at date is
a datetime.datetime obj](https://github.com/sigmavirus24/github3.py/blob/3bb730f11a70ab84f9f64835442dc4c9c62638ea/src/github3/repos/repo.py#L2938-L2941).

This makes me wonder how we're getting a string.

Signed-off-by: jmeridth <[email protected]>
@jmeridth jmeridth marked this pull request as draft April 2, 2024 06:15
@jmeridth
Copy link
Member

jmeridth commented Apr 2, 2024

@zkoppert I pushed up tests. I need to debug some more. The repo.created_at should not be a string from the github3.py library. It should be a datetime.datetime object

@jmeridth
Copy link
Member

jmeridth commented Apr 2, 2024

Have confirmed locally, the repo.created_at variable is a string. 😕

Looking at a couple more things and then will open this up for review and merge.

repo.created_at is of type github3.repos.repo.ShortRepository which
seems to have a created_at attribute of type string

github3.repos.repo.Repository has a parsed created_at type
of datetime.datetime object but that is not what we get back
in collections

Signed-off-by: jmeridth <[email protected]>
@jmeridth jmeridth marked this pull request as ready for review April 2, 2024 14:48
@jmeridth
Copy link
Member

jmeridth commented Apr 2, 2024

repo.created_at is of type github3.repos.repo.ShortRepository which
seems to have a created_at attribute of type string

github3.repos.repo.Repository has a parsed created_at type
of datetime.datetime object but that is not what we get back
in collections

@jmeridth jmeridth merged commit 1cac70e into main Apr 2, 2024
19 checks passed
@jmeridth jmeridth deleted the fix-TZ branch April 2, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: TypeError when replacing TZ info
2 participants