Remove need for lodash/bind; Optimize

[dhritzkiv]

• Update ampersand-events (which incorporates lodash updates)
• Update lodash to v4, using single module vs. invidual modules. Correct argument pattern for lodash.forOwn (this is no longer a third argument?)
• Remove lodash.bind; use native .bind. Previously required for phantomjs (see next point)
• Update phantomjs to v2, which includes .bind. Installs using recommended phantomjs-prebuilt
• Update coveralls, tape, zuul
• Reuse similar code (at L105)

[pgilad]

Great job! BTW, all the lodash includes are case-sensitive (depending on OS/FS but ubuntu is), meaning var isString = require('lodash/isstring'); will fail in ubuntu but pass in osx. You need to reference the exact file: var isString = require('lodash/isString');

[dhritzkiv]

Good catch. I totally forgot about that behaviour.

[coveralls]

Coverage decreased (-96.5%) to 1.269% when pulling 5da5629b25509952108ee0ca8a33ce5a5798b81b on dhritzkiv:update-deps into 7021f00b35cf0d328749d0898e90f9670691a890 on AmpersandJS:master.

[coveralls]

Coverage decreased (-0.006%) to 97.716% when pulling 5c05238100291959a181eb531d6d3eb95ab69ac3 on dhritzkiv:update-deps into 7021f00b35cf0d328749d0898e90f9670691a890 on AmpersandJS:master.

[coveralls]

Coverage decreased (-0.006%) to 97.716% when pulling 5c05238100291959a181eb531d6d3eb95ab69ac3 on dhritzkiv:update-deps into 7021f00b35cf0d328749d0898e90f9670691a890 on AmpersandJS:master.

[wraithgar]

You'll likely need to rebase this PR against master, there was a bugfix PR from last week I just merged.

[coveralls]

Coverage decreased (-0.006%) to 97.733% when pulling 479cb57daa75146828f2172478f3285786675ad2 on dhritzkiv:update-deps into b27f7d300b397b8c8525f45c3e74b81d3bd9cf47 on AmpersandJS:master.

[wraithgar]

This looks good to me, Let's not merge this until I've published a patch release from pr #235 so we don't unnecessarily couple this lodash update with that bugfix

ETA: it's published, I'm good on this shipping whenever.

[dhritzkiv]

I saw that ampersand-view was updated recently to v10 to include lodash@v4 as well as ampersand-state@v5. However, ampersand-state@v5 still uses older lodash versions (addressed via this PR). When this gets a merge + release, ampersand-view will need its ampersand-state dependency updated, which will mean a new major version.

[cdaringe]

are we sure that it would be a major? no breaking changes would occur. dep swaps would occur, but no functionality or API change would be expected. IMHO, it's a patch, unless there's something im missing from the &-events bump.

[dhritzkiv]

That's a fair point.

I suppose my thinking was: ampersand-state is a critical part of ampersand-view, and any major version bumps of -state (other ampersand modules received a major update for the lodash upgrade) would require a major version bump of -view. But behaviourally, this PR shouldn't break anything.

[coveralls]

Coverage decreased (-0.006%) to 97.733% when pulling 400e13638a9721cfcadb6d4e9f1887c9da496892 on dhritzkiv:update-deps into 987cfbeceb3dd45da7b3b03e6608cd83739faf85 on AmpersandJS:master.

[dhritzkiv]

Any update on this PR?

[cdaringe]

looks good to me! loving all the 🔴 lines. :) @wraithgar's PR is merged, so hopefully he's cool with proceeding now.

[coveralls]

Coverage decreased (-0.006%) to 97.733% when pulling ae918e08f462b834e22bb2a0a1e5df71aa935bea on dhritzkiv:update-deps into 987cfbeceb3dd45da7b3b03e6608cd83739faf85 on AmpersandJS:master.

[coveralls]

Coverage decreased (-0.006%) to 97.733% when pulling 0e968b54c5635ad40dcb91431ab44e574b4e3b29 on dhritzkiv:update-deps into 49831e5820103ea6eacb2c4d847a99f485a7f80f on AmpersandJS:master.

[dhritzkiv]

I merged in master, so the main changes are now:

• removing the requirement for lodash's bind method and using native bind
  • accomplished by using an updated version of phantom that includes support for Function.prototype.bind
• DRYer functions that simplify setting the value from serialised children or collections

[dhritzkiv]

Bump. The tests are actually passing (see the test details) and the coverage decrease is minimal. Seems this got lots of thumbs up previously. Good to merge?

[wraithgar]

Yes, great!