Skip to content

Conversation

sproberts92
Copy link
Contributor

When building Yajl with the default build settings on Windows the resultant library is yajl.dll. I added an attempt to load this in load_yajl, then did a little refactoring to reduce code duplication.

I did the addition and the refactoring in separate commits, so I'm happy to resubmit this pull request without the refactor if you don't like it.

Passes all tests on Linux (Debian). Doesn't pass tests on Windows but only due to line ending differences. I can get almost all to pass by playing with the encoding when loading the .gold cases (except some with unusual characters such as escaped_bulgarian.json.gold).

Yajl built with default build settings yields yajl.dll, so make an
attempt to load this.

At the moment, the existing load structure has simply been duplicated
to achieve this. This could be refactored into something cleaner.
To reduce code duplication introduced by adding windows support in
commit 117166f.

List of file names is generated using a list comprehension so that
the unique windows file name can be appended. Consequently, the
list of file names can be iterated over and two separate cases are
not needed.
@coveralls
Copy link

coveralls commented Jul 2, 2016

Coverage Status

Coverage remained the same at 92.032% when pulling 89ef9a4 on sproberts92:master into 6917b7d on pykler:master.

@pykler pykler merged commit e8fb5cc into pykler:master Jul 5, 2016
@pykler
Copy link
Owner

pykler commented Jul 5, 2016

Thanks!

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.

3 participants