krakenjs / lusca

Application security for express apps.
Other
1.78k stars 123 forks source link

Remove dependency and add grunt-cli #84

Closed geek closed 7 years ago

geek commented 8 years ago
indutny commented 8 years ago

What is a reason for the removal of core-util-is? Why was it added in first place?

geek commented 8 years ago

@indutny Removing because it seems like overkill for 3 type checks. The only issue I see with this PR is that if need to support old versions of node (pre 0.8.0) that don't have Array.isArray

indutny commented 8 years ago

@geek I doubt there is any reason to support pre 0.8.0 nowadays.

Please change package.json, though ;)

πŸ‘

grawk commented 8 years ago

Seems fine. I'll give anyone a chance to chime in as far as installing a local copy of grunt. This is a change that we'd want to make all over if we're going to start making it.

jasisk commented 8 years ago

Cool on all things.

core-util-is usage was more of a common pattern amongst our modules than it is a matter of practical usage. This change is a bit change for changes sake but still practical.

grunt-cli has been added in some of our other reposβ€”lusca has just been stuck in the prefer global mindset from nearly four years ago. lusca is amongst the top offenders as far as avoiding spring-cleaning has gone. πŸ˜€

linkRace commented 7 years ago

Seems like this has support to merge? Will get one more from my team.

subeeshcbabu-zz commented 7 years ago

πŸ‘