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

Fix reading of binary metadata file #3845

Merged
merged 1 commit into from Jul 16, 2015

Conversation

Projects
None yet
5 participants
@fw42
Contributor

fw42 commented Jul 9, 2015

Looks like Windows is having problems with reading binary files using File#read. Not sure why this works on Linux and Mac OS X.

Closes #3842 (hopefully).

@mmistakes or @envygeeks, could you confirm that this fixes the problem?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 9, 2015

Member

Something wrong with Cucumber... Was it recently updated?

Member

parkr commented Jul 9, 2015

Something wrong with Cucumber... Was it recently updated?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 9, 2015

Contributor

That's ultra weird, all our tests are passing but it fails after the test...

Contributor

envygeeks commented Jul 9, 2015

That's ultra weird, all our tests are passing but it fails after the test...

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Jul 10, 2015

Is there any easy way to test this? I'm familiar with using prerelease gems but not really sure how to go about pulling this fix in to verify it works on Windows.

mmistakes commented Jul 10, 2015

Is there any easy way to test this? I'm familiar with using prerelease gems but not really sure how to go about pulling this fix in to verify it works on Windows.

@fw42

This comment has been minimized.

Show comment
Hide comment
@fw42

fw42 Jul 10, 2015

Contributor

Go to my branch here: https://github.com/fw42/jekyll/tree/fix_binread and then click the "Download as ZIP" button on the right. After unzipping, you can run ruby bin\jekyll I guess.

Contributor

fw42 commented Jul 10, 2015

Go to my branch here: https://github.com/fw42/jekyll/tree/fix_binread and then click the "Download as ZIP" button on the right. After unzipping, you can run ruby bin\jekyll I guess.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 10, 2015

Member

mmistakes, I use gem "jekyll", github: "fw42/jekyll", branch: "fix_binread" in my Gemfile.

Member

parkr commented Jul 10, 2015

mmistakes, I use gem "jekyll", github: "fw42/jekyll", branch: "fix_binread" in my Gemfile.

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Jul 10, 2015

Thanks @parkr, adding that to my Gemfile worked like a charm. I'll do a more thorough test in a bit but initial and incremental builds on a new Jekyll site seem to be working. The marshal data error no longer triggers.

Will run it through an existing site to verify all is well.

mmistakes commented Jul 10, 2015

Thanks @parkr, adding that to my Gemfile worked like a charm. I'll do a more thorough test in a bit but initial and incremental builds on a new Jekyll site seem to be working. The marshal data error no longer triggers.

Will run it through an existing site to verify all is well.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 10, 2015

Member

Thanks, @mmistakes!

Member

parkr commented Jul 10, 2015

Thanks, @mmistakes!

@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Jul 10, 2015

Looks like the fix is a keeper. Just tried it on one of my larger sites and everything worked as it should. Love the new incremental build feature and --profile flag. Keep up the great work!

mmistakes commented Jul 10, 2015

Looks like the fix is a keeper. Just tried it on one of my larger sites and everything worked as it should. Love the new incremental build feature and --profile flag. Keep up the great work!

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 15, 2015

Contributor

If @parkr thinks it's fine I'll merge it after I find out what I broke when I updated our cucumber which I plan on doing this afternoon.

Contributor

envygeeks commented Jul 15, 2015

If @parkr thinks it's fine I'll merge it after I find out what I broke when I updated our cucumber which I plan on doing this afternoon.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 15, 2015

Member

If @parkr thinks it's fine I'll merge it after I find out what I broke when I updated our cucumber which I plan on doing this afternoon.

Looks good to me. Thanks, @envygeeks. 👍

Member

parkr commented Jul 15, 2015

If @parkr thinks it's fine I'll merge it after I find out what I broke when I updated our cucumber which I plan on doing this afternoon.

Looks good to me. Thanks, @envygeeks. 👍

envygeeks added a commit that referenced this pull request Jul 16, 2015

Merge pull request #3845 from fw42/fix_binread
Fix reading of binary metadata file

@envygeeks envygeeks merged commit 48d6eea into jekyll:master Jul 16, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.