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

Use platform.uname instead of os.uname #1044

Closed
wants to merge 2 commits into from
Closed

Conversation

Tontyna
Copy link
Contributor

@Tontyna Tontyna commented Jan 29, 2020

platform.uname() should work on all platforms (cf. #1023 (comment)) and therefore weasyprint --info, too.

Decided to import platform locally to avoid unnecessary initialization when there is no --info argument.

It's just educated guessing that it reveals interesting facts on Windows -- I don't have no Windows computer anymore.

Have no idea how to write a unit test for PrintInfo action 😳

@liZe
Copy link
Member

liZe commented Jan 29, 2020

Hello!

I had actually changed this in the clean branch too, in 10a7863. I really should have merged it sooner… And there’s also a test in this commit, at least checking that it doesn’t crash as it used to do on Windows 😄.

@Tontyna
Copy link
Contributor Author

Tontyna commented Jan 29, 2020

Ah so simple, just _run('--info')

@Tontyna Tontyna closed this Jan 29, 2020
@liZe
Copy link
Member

liZe commented Jan 29, 2020

I’ve merged the clean branch.

It's just educated guessing that it reveals interesting facts on Windows -- I don't have no Windows computer anymore.

Oo

@Tontyna
Copy link
Contributor Author

Tontyna commented Jan 29, 2020

Oo

I know, Linux sucks, but Windows is an abomination.

@Tontyna Tontyna deleted the printinfo branch January 29, 2020 23:17
@liZe
Copy link
Member

liZe commented Jan 31, 2020

I know, Linux sucks, but Windows is an abomination.

That was interesting reading!

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.

2 participants