Skip to content

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

Merged
jmeridth merged 4 commits intomainfrom
fix-TZ
Apr 2, 2024
Merged

fix: remove time of day and tz info properly#83
jmeridth merged 4 commits intomainfrom
fix-TZ

Conversation

@zkoppert
Copy link
Copy Markdown
Collaborator

@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

Signed-off-by: Zack Koppert <zkoppert@github.com>
@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
Copy Markdown
Collaborator Author

zkoppert commented Apr 2, 2024

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

Copy link
Copy Markdown
Collaborator

@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 <jmeridth@gmail.com>
@jmeridth jmeridth marked this pull request as draft April 2, 2024 06:15
@jmeridth
Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Collaborator

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 <jmeridth@gmail.com>
@jmeridth jmeridth marked this pull request as ready for review April 2, 2024 14:48
@jmeridth
Copy link
Copy Markdown
Collaborator

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

Signed-off-by: jmeridth <jmeridth@gmail.com>
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