-
Notifications
You must be signed in to change notification settings - Fork 146
Fix build failure for Travis, Appveyor and Circle #36 #37
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
Conversation
Python3 returns 'bytes' and not 'str' when reading/writing to/from a binary stream. So, the files need to be opened in text mode for `str` related operations. Signed-off-by: Yash Nisar <[email protected]>
@pombredanne A quick review possible ? |
Circle CI states that:
Lets see if running |
Signed-off-by: Yash Nisar <[email protected]>
@a-h-i @sschuberth Would you like to have a look ? |
@@ -1,5 +1,6 @@ | |||
machine: | |||
pre: | |||
- brew update |
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.
Please explain in the commit message why this is necessary, because AFAIK brew install
does an auto-update.
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.
Some useful threads which I found were -> https://discuss.circleci.com/t/homebrew-must-be-run-under-ruby-2-3-runtimeerror/17232 and Homebrew/brew#3299 (comment)
temp_dir = tempfile.mkdtemp(prefix='test_spdx') | ||
result_file = os.path.join(temp_dir, 'spdx-simple.tv') | ||
with open(result_file, 'wb') as output: | ||
with open(result_file, 'w') as output: |
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.
Sorry, I'm not a Python expert: Will this work for Python 2, too? Because we need to stay compatible to both versions 2 and 3.
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.
Yes, this will work on Python 2 too. The tests pass for 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.
Then I'm wondering why it ever worked for Python 3. Do you have any idea about that?
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.
Yes, Python 3 returns 'bytes' and not 'str' when reading/writing to/from a binary stream. So, the files need to be opened in text mode for str
related operations.
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.
@sschuberth I guess the answer is in the Travis Builds - If you look at older builds that passed the tests hadn't been added up till then (back when the builds were passing that is)
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.
Oh, I didn't realize @pombredanne had committed this directly, without going through a PR that runs CI. Otherwise that change should have never gotten in.
Superseded by #38. |
Python3 returns 'bytes' and not 'str' when reading/writing
to/from a binary stream. So, the files need to be opened in
text mode for
str
related operations.Signed-off-by: Yash Nisar [email protected]
Closes #36