search

heavysixer / d4

A friendly reusable charts DSL for D3
MIT License
433 stars 46 forks source link

changed scope of d4 var to work with how browserify compiles #45

Closed lomaxap closed 8 years ago

heavysixer commented 8 years ago

Did you manually change the references to d4 in the compiled file? Because if so that won't work with the way the lib is compiled from parts.

lomaxap commented 8 years ago

no, i only changed the base.js file and compiled with grunt

heavysixer commented 8 years ago

Ok the compiled file looked a bit odd. Let me test your branch locally but if everything works i'll make a new release. Thanks!

heavysixer commented 8 years ago

One final question, is there a way to give a minimal browserify test for this? How did you confirm this worked?

lomaxap commented 8 years ago

good question. I found this tool browserify-test and ran this test which passed:

var expect = require( 'chai' ).expect;

describe('browserify test', function() {
  it('browserify should compile d4', function() {
       var d4 = require( '../lib/d4.js' );
      expect(d4).to.not.be.an('undefined');
  });
});

let me know if that works.

heavysixer commented 8 years ago

Awesome, can you integrate that with the existing test suite and append the PR you sent me?

Thanks again for your hard work!

Best Regards, Mark

On Mar 29, 2016, at 10:47 AM, Andrew Lomax notifications@github.com wrote:

good question. I found this tool browserify-test https://github.com/alekseykulikov/browserify-test and ran this test which passed:

var expect = require( 'chai' ).expect;

describe('browserify test', function() { it('browserify should compile d4', function() { var d4 = require( '../lib/d4.js' ); expect(d4).to.not.be.an('undefined'); }); }); let me know if that works.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/heavysixer/d4/pull/45#issuecomment-202968779

lomaxap commented 8 years ago

ok. no problem. I need to add grunt-browserify and a new task to run during grunt test. is that fine with you?

heavysixer commented 8 years ago

Perfect, I’ll confirm it works for me and then merge.

On Mar 29, 2016, at 12:04 PM, Andrew Lomax notifications@github.com wrote:

ok. no problem. I need to add grunt-browserify and a new task to run during grunt test. is that fine with you?

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/heavysixer/d4/pull/45#issuecomment-203000163