favicon middleware integration

carrot / charge

A bundle of useful middleware and tools for serving static sites

Other

22 stars 1 forks source link

favicon middleware integration #19

Closed kylemac closed 9 years ago

kylemac commented 10 years ago

[x] integrates serve-favicon middleware
[x] tests

admittedly the test is pretty weak, but i'm not really sure how else to test it - so I just added a test that makes sure that by defining a custom favicon it doesn't break a normal html request. I'm open to suggestions.

/cc @joshrowley @jenius

coveralls commented 10 years ago

Coverage increased (+0.01%) when pulling 420d22be5abc94689f4cb5f155a4a0e3b9a41d44 on favicon into 4317ca0d562c68b32169f87e15e085e400a6fc22 on master.

jescalan commented 10 years ago

Is there a default favicon slash will it serve favicon.ico from the root if none are provided in the options?

coveralls commented 10 years ago

Coverage increased (+0.01%) when pulling 420d22be5abc94689f4cb5f155a4a0e3b9a41d44 on favicon into 4317ca0d562c68b32169f87e15e085e400a6fc22 on master.

kylemac commented 10 years ago

currently if you don't pass a favicon in your options, we don't use the serve-favicon middleware and it defaults to your browsers behavior which would look for favicon.ico in root.

I decided against having our own charge default favicon - but I could be swayed.

notslang commented 10 years ago

Migrating this from a discussion on slack:

[Adding serve-favicon] sounds like a bad idea... when I look at a new project I expect the favicon to be at the root. It's kinda an important convention. If someone really needs it to be in a different place then they should just symlink it or use a redirect. And for caching: wouldn't a generic caching mechanism suffice?

kylemac commented 9 years ago

we haven't been looking for this feature in the last year, so yagni for now.