search

anacronw / multer-s3

multer storage engine for amazon s3
MIT License
660 stars 190 forks source link

Add support for setting Cache-Control headers #37

Closed lukechilds closed 8 years ago

lukechilds commented 8 years ago

Fixes #36

LinusU commented 8 years ago

LGTM

Can you add documentation as well?

lukechilds commented 8 years ago

Yeah, sure 👍

lukechilds commented 8 years ago

@LinusU done :)

LinusU commented 8 years ago

Awesome, thanks for your contribution

LGTM

anacronw commented 8 years ago

looks good, but I'm honestly not very happy w/ the entire chain of callbacks we're tacking onto it - that's outside of this PR

LinusU commented 8 years ago

@badunk I'll submit a pull request to fix that as soon as this is merged, sounds good?

anacronw commented 8 years ago

oh yeah for sure, thanks @LinusU

lukechilds commented 8 years ago

Yeah I agree, I threw up in my mouth a little when I saw that, I was just following the convention 😆

It could still be trimmed down quite a bit after your latest PR.

Also, just a suggestion, but instead of manually implementing all the stuff you wanna pass through to S3 (metadata, cache control, acl, content type etc) it might be worth having one s3Params property that gets merged with the default S3 param object. That way people can add any options that the S3 lib supports without requiring any extra modifications to this lib.