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

Support for Py3k #21

Merged
merged 7 commits into from Mar 5, 2014
Merged

Support for Py3k #21

merged 7 commits into from Mar 5, 2014

Conversation

Alexey-T
Copy link
Contributor

@Alexey-T Alexey-T commented Mar 2, 2014

No description provided.

@pwaller
Copy link
Owner

pwaller commented Mar 2, 2014

Nice work, though the tests failed, could you fix that?

Also, do you need to from __future__ import print_function? It would also be good to edit the travis config to include the python versions you'd like to test on.

@Alexey-T
Copy link
Contributor Author

Alexey-T commented Mar 2, 2014

Can u tell, how to make "test"? i don't know travis. what to install in pip, what to run

@pwaller
Copy link
Owner

pwaller commented Mar 2, 2014

You should be able to run PYTHONPATH=. python pyfiglet/test.py. Does it not work?

@Alexey-T
Copy link
Contributor Author

Alexey-T commented Mar 2, 2014

almost. I have error

G:\pyfiglet>python pyfiglet\test.py
Traceback (most recent call last):
  File "g:\py33\lib\subprocess.py", line 1105, in _execute_child
startupinfo)
FileNotFoundError: [WinError 2] Не удается найти указанный файл

during run of test.py.

@pwaller
Copy link
Owner

pwaller commented Mar 2, 2014

Ahh, you're on Windows. It's trying to invoke the original figlet and toilet to compare the results. I can't easily help you run them unless you know how to install these on windows.

@Alexey-T
Copy link
Contributor Author

Alexey-T commented Mar 3, 2014

So what about it

@pwaller
Copy link
Owner

pwaller commented Mar 3, 2014

Apologies @Alexey-T, your help is very much appreciated! Please be patient since I'm very busy at the moment and want to review it reasonably, hope you don't mind! :)

Please do ping me again if you don't hear from me in a few days.

@pwaller
Copy link
Owner

pwaller commented Mar 4, 2014

Hi, Just had a look, looks good, except that the tests fail on python 3:

https://travis-ci.org/pwaller/pyfiglet/jobs/19937917#L463

In test.py it just needs to be updated:

diff --git a/pyfiglet/test.py b/pyfiglet/test.py
diff --git a/pyfiglet/test.py b/pyfiglet/test.py
index 3812fe5..8af839c 100755
--- a/pyfiglet/test.py
+++ b/pyfiglet/test.py
@@ -45,7 +45,7 @@ def main():
             raise Exception('Missing font file: '+fontpath)

         p = Popen(cmd, bufsize=1,stdout=PIPE)
-        outputFiglet = p.communicate()[0]
+        outputFiglet = p.communicate()[0].decode("ascii")

         if outputPyfiglet == outputFiglet:
             print('[OK] %s' % font)

Then I think it's good for merging.

@Alexey-T
Copy link
Contributor Author

Alexey-T commented Mar 4, 2014

Added this

pwaller added a commit that referenced this pull request Mar 5, 2014
@pwaller pwaller merged commit 7014d73 into pwaller:master Mar 5, 2014
@stefanor stefanor mentioned this pull request Jul 27, 2014
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.

None yet

2 participants