Skip to content

Conversation

@ndhoule
Copy link
Contributor

@ndhoule ndhoule commented Jun 17, 2015

cc @boneskull @jbnicolai

Resolves #1728.

Depends on merging #1727 because it pulls in an npm dependency; uses style from #1750

@boneskull
Copy link
Member

I think I'd rather go with LoDash here, since there's going to be a ton of other shims we can replace it with. @mochajs/mocha thoughts?

@ndhoule
Copy link
Contributor Author

ndhoule commented Jun 30, 2015

@boneskull Which lodash function would you use here? _.create?

@ndhoule ndhoule closed this Jul 5, 2015
@ndhoule ndhoule force-pushed the refactor/remove-dunderproto branch from a7cf327 to d5ddcdc Compare July 5, 2015 01:36
@ndhoule ndhoule reopened this Jul 5, 2015
@ndhoule
Copy link
Contributor Author

ndhoule commented Jul 5, 2015

@boneskull Refactored this to use lodash.create, which can easily be swapped out for a full lodash when necessary. I think this version is less expressive than the inherits version but meh

@boneskull
Copy link
Member

@ndhoule why do you feel it's less expressive?

@jbnicolai jbnicolai force-pushed the master branch 3 times, most recently from 2f458ab to 2952eca Compare July 5, 2015 10:25
@jbnicolai
Copy link

@nathanboktae I agree
@boneskull create(..., {constructor: ...}) seems less expressive than inherit(..., ...).

On the other hand, since we're intending to go with lodash anyways, I say we merge this version :)

Perhaps to make this a bit nicer we could refactor it into a utility method in Base

function inherit(prototype, constructor) {
  return create(prototype, {
    constructor: constructor
  });
}

But no strong feelings about that.

@ndhoule
Copy link
Contributor Author

ndhoule commented Jul 5, 2015

haha pretty much exactly what @jbnicolai said, but really I don't care—I say merge it as is, it's certainly better than the __proto__ version

@jbnicolai
Copy link

Agree, still a huge improvement 👍

jbnicolai pushed a commit that referenced this pull request Jul 5, 2015
Replace __proto__ references with inherits module
@jbnicolai jbnicolai merged commit 2b2c576 into mochajs:master Jul 5, 2015
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