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

Format-dates #31

Merged
merged 7 commits into from
Sep 12, 2021
Merged

Format-dates #31

merged 7 commits into from
Sep 12, 2021

Conversation

bpepple
Copy link
Member

@bpepple bpepple commented Sep 11, 2021

  • Fix date formatting for creator date of death
  • Use Meta class to handle primary date formats

@Buried-In-Code
Copy link
Member

While we're updating the dates I think it'd also be good to change the DoB and DoD to be Date format instead of Datetime.

@Buried-In-Code Buried-In-Code linked an issue Sep 12, 2021 that may be closed by this pull request
@Buried-In-Code
Copy link
Member

Also the tests should pass if you include the Github Secret COMICVINE_API_KEY to your fork config.

@bpepple
Copy link
Member Author

bpepple commented Sep 12, 2021

Also the tests should pass if you include the Github Secret COMICVINE_API_KEY to your fork config.

Thought I'd set it, but I'll check it again on my fork.

@bpepple
Copy link
Member Author

bpepple commented Sep 12, 2021

While we're updating the dates I think it'd also be good to change the DoB and DoD to be Date format instead of Datetime.

Not a bad idea. Give me a minute and I'll push a new commit to change that.

@bpepple
Copy link
Member Author

bpepple commented Sep 12, 2021

Looks like my GH Secret Key is still not working. Really, I think an expiring testing cache/env secrets isn't that great of a solution for testing, since it sort of defeats the purpose of having a testing cache if it's just going to expire every 14 days.

Beyond the whole everyone who forks it needing to setup a GH secret hassle, you run into the potential problem with the data from ComicVine possibly changing due to editing. I'd lean towards either using a non-expiring testing cache, or uses fixtures for the data.

@Buried-In-Code
Copy link
Member

That's true, I guess my implementation doesn't work for tests as I'd planned. I'll take a look at my implementation in another PR and make it possible to not expire the cache for tests

@Buried-In-Code Buried-In-Code merged commit 9072f9d into Metron-Project:main Sep 12, 2021
@bpepple bpepple deleted the format-dates branch September 13, 2021 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datetime Validation Errors
2 participants