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

Iso time #52

Merged
merged 32 commits into from
Sep 27, 2022
Merged

Iso time #52

merged 32 commits into from
Sep 27, 2022

Conversation

joesmolsky
Copy link
Collaborator

Change time format to ISO format. This makes the TimeStuff class obsolete. Times can now be set with datetime.utcnow().isoformat().

Version also changes from 1.1.0 to 1.1.1

@joesmolsky joesmolsky added the enhancement New feature or request label Aug 5, 2022
Copy link
Collaborator

@KaraMelih KaraMelih left a comment

Choose a reason for hiding this comment

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

Looks quite good, thanks for taking care of it!
I just want to check it with pycharm to see if the TimeStuff is not used in anywhere else.
This indeed simplifies things

@KaraMelih
Copy link
Collaborator

Your commits from yesterday started to fail the tests;
https://github.com/SNEWS2/SNEWS_Publishing_Tools/actions
could you have a look at the cause

snews_pt/snews_pub.py Outdated Show resolved Hide resolved
@joesmolsky
Copy link
Collaborator Author

I have updated the format checker to include warnings with the logs.

Also did some fixes to the time checking logic and fixed some typos in the test function times.

@KaraMelih
Copy link
Collaborator

Thanks Joe it looks better!
I can try to test it tomorrow, latest next Monday and merge it.
Please let me know if you need me to do it faster

Copy link
Collaborator

@KaraMelih KaraMelih left a comment

Choose a reason for hiding this comment

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

I made some comments. I noticed that there are a few minor things need to be changed

snews_pt/snews_pt_utils.py Outdated Show resolved Hide resolved
snews_pt/snews_pt_utils.py Show resolved Hide resolved
@KaraMelih
Copy link
Collaborator

You can also debug these via snews_pt run-scenarios it shows already these error messages without having to interact with cs

@joesmolsky joesmolsky merged commit b5fb57f into main Sep 27, 2022
@joesmolsky joesmolsky deleted the iso-time branch September 27, 2022 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants