search

ember-cli / broccoli-asset-rev

Broccoli plugin to add fingerprint checksums and CDN URLs to your assets
MIT License
87 stars 84 forks source link

hashing may be funky #70

Closed stefanpenner closed 8 years ago

stefanpenner commented 8 years ago

https://github.com/rickharrison/broccoli-asset-rev/blob/master/lib/fingerprint.js#L81 is from: https://github.com/rickharrison/broccoli-asset-rev/blob/master/lib/fingerprint.js#L35 which is: https://github.com/rickharrison/broccoli-asset-rev/blob/master/lib/fingerprint.js#L181

but md5hash takes only 1 arg, but https://github.com/rickharrison/broccoli-asset-rev/blob/master/lib/fingerprint.js#L81 passes two, the second arg is lost.

we likely want:

var hash = this.hashFn(file + '\x00' + tmpPath);

or to remove the second arg, or leave a comment explaining this was the intent (if the default functionality is to ignore the path, but the custom hashFn could use it)

rickharrison commented 8 years ago

That argument is only there for users who want to have a custom hash function. The default md5 implementation doesn't use it. It is from #58

stefanpenner commented 8 years ago

Sounds good. It should likely be commented, i can submit a pr later this weekend