Don't allow extending a protected callback.

fjorgemota / jimple

Just a dependency injection container to NodeJS and to the browser using new ES6 features

MIT License

75 stars 12 forks source link

Don't allow extending a protected callback. #4

Closed alxarch closed 8 years ago

alxarch commented 8 years ago

A protected callback is supposed to act like a parameter. It should not be possible to extend such a function because te current code would call it with container as first argument which beats the purpose of 'protecting' it in the first place.

fjorgemota commented 8 years ago

Nice catch!

Some observations:

[x] A new test to test this new behavior is welcome, too;
[x] The README should be adjusted to alert about this new behavior;

With this fixes, I think that should be acceptable to release version 1.4.1.

What you think?

PS: I will try to investigate Travis CI errors soon.

alxarch commented 8 years ago

I just added a test on the same commit. It passes locally. About the message it's up to you, in my head a protected callback is the same as a parameter.

About Travis maybe you should point npm scripts to ./node_modules/istanbul/bin/istanbul instead of just istanbul. IIRC travis has no 'global' packages preinstalled

Cheers, Alex

fjorgemota commented 8 years ago

NIce!

And nice tip too, however, the error seems related to Google Chrome problems in Precise distro, as related in travis-ci/travis-ci#5899

I am going to merge the changes soon after some fixes related to Travis.

Thanks!

fjorgemota commented 8 years ago

About istanbul...seems like npm automatically adds to PATH the path to bin directories of node modules installed as dependencies..see https://docs.npmjs.com/misc/scripts#path

Anyway. Merged! I will adjust the message and README soon. :)

Thank you!