search

HenrikJoreteg / moonboots

A set of conventions and tools for bundle and serving clientside apps with node.js
158 stars 20 forks source link

Upgrade to browserify 9 #62

Closed latentflip closed 9 years ago

latentflip commented 9 years ago

So I followed the work @lukekarrys had done for browserify 5, and it worked basically the same. Some inline comments/questions below.

latentflip commented 9 years ago

So I think there are two questions that remain in this, with regards to the consistent hashing behaviour discussion from before:

  1. Have the inconsistencies in hashing now gone away in browserify (luke seems to think so) meaning that we can safely remove the mdeps stuff, as I have done
  2. Do these tests actually pass under windows, given that it has different line endings, and thus, the hardcoded expected hashes in the tests may not match the hashes generated for the test files under windows?

Maybe @nlf has thoughts on these?

lukekarrys commented 9 years ago

Have the inconsistencies in hashing now gone away in browserify (luke seems to think so) meaning that we can safely remove the mdeps stuff, as I have done

I couldn't ever get the test in tests/consistentHash.js to fail, but that doesn't mean there isn't still a bug :smile: But my feeling is that it is safe to remove. Maybe a more complex test bundle would generate fails?

Do these tests actually pass under windows, given that it has different line endings, and thus, the hardcoded expected hashes in the tests may not match the hashes generated for the test files under windows?

I'm willing to bet that they do not pass under windows. And the discussion in #50 is probably sill relevant. We could try something where we replace all the line endings manually and see if the hash is the same across machines/OSes. We could also use travis as a linux system for this. But overall I don't think this is a blocker to getting this PR merged.

latentflip commented 9 years ago

Right of course because those failures exist in the current version anyway.

Philip Roberts

On 17 Dec 2014, at 16:00, Luke Karrys notifications@github.com wrote:

Have the inconsistencies in hashing now gone away in browserify (luke seems to think so) meaning that we can safely remove the mdeps stuff, as I have done

I couldn't ever get the test in tests/consistentHash.js to fail, but that doesn't mean there isn't still a bug

Do these tests actually pass under windows, given that it has different line endings, and thus, the hardcoded expected hashes in the tests may not match the hashes generated for the test files under windows?

I'm willing to bet that they do not pass under windows. And the discussion in #50 is probably sill relevant. We could try something where we replace all the line endings manually and see if the hash is the same across machines/OSes. We could also use travis as a linux system for this. But overall I don't think this is a blocker to getting this PR merged.

— Reply to this email directly or view it on GitHub.

lukekarrys commented 9 years ago

Added a commit to update browserify to the latest v8.

Also added a commit to fix the through transform to actually write the data to the bundle. Didn't affect any tests but might've affected them in the future (and it did make me scratch my head for a bit while I was working on #63).

@latentflip Do you think we should merge this and release a major version?

lukekarrys commented 9 years ago

Updated browserify to version 9 now.

nlf commented 9 years ago

:+1: