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

Add encoding option + fixed test that fails on windows. #238

Merged
merged 5 commits into from Oct 30, 2018

Conversation

@Zwork101
Copy link
Contributor

Zwork101 commented Oct 1, 2018

To solve one of the issues presented in #222, the very simple option was allow the encodings kwarg to be passed in. I also added a test for it. After running the tests, I noticed test_table_repr failed. I believe this is because on linux based machines, it displays the memory location with lowercase letters, while windows uses uppercase and lowercase.

@Zwork101

This comment has been minimized.

Copy link
Contributor Author

Zwork101 commented Oct 1, 2018

I'm going to work on this some more, I just need to use io.open instead. I'm not great at this cross-compatibility stuff.

Zwork101 added 2 commits Oct 1, 2018
…s should be the last commit (please test gods).
Copy link
Owner

msiemens left a comment

Thanks for the PR, I've got one change request, otherwise it looks good 🙂

tinydb/storages.py Outdated Show resolved Hide resolved
@msiemens

This comment has been minimized.

Copy link
Owner

msiemens commented Oct 26, 2018

@Zwork101 Could you have a look at the code review?

@Zwork101

This comment has been minimized.

Copy link
Contributor Author

Zwork101 commented Oct 26, 2018

Oh, you're right sorry. Was going to deal with this a while ago and forgot. I'll write a note to do it later right now.

@msiemens msiemens merged commit ba68839 into msiemens:master Oct 30, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@msiemens

This comment has been minimized.

Copy link
Owner

msiemens commented Oct 30, 2018

No worries! Thanks for your contribution ❤️

@msiemens

This comment has been minimized.

Copy link
Owner

msiemens commented Nov 6, 2018

This is now released in v3.12.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.