-
Notifications
You must be signed in to change notification settings - Fork 80
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] Iso format for v3 is called twice #49
Conversation
.travis.yml
Outdated
@@ -1,7 +1,6 @@ | |||
sudo: false | |||
language: python | |||
python: | |||
- "2.7" | |||
- "3.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why removing the 2.7 version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be able to pass the tests
return datetime.now(timezone.utc).isoformat()[:-13]+'Z' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that looks good, now that I look at the other code, it was indeed a strange setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lparkerlm can you weigh in on the removal of python 2.7?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flachica we had a discussion with @lparkerlm and as it stands, removing python 2.7 cannot be accepted as part of this PR. We need more time to flesh out the issue with this version, and decide if we maintain or remove support.
In my opinion your code looks ok, and if it works with Python 3, then I think it's good enough for the current purpose. Python 2.7 issue has been lingering for a while, so we would need to take a proper look at it.
As of this issue blockchain-certificates#48 and the PR blockchain-certificates#49 (paused for Python3 concerns) I proposte this PR to only put the correct datetime format without any Python3 upgrade
Updated without Python3 removal in #51 |
No description provided.