expressjs / expressjs.com

https://expressjs.com
Other
5.25k stars 1.46k forks source link

asset optimizations #1489

Closed MehfoozurRehman closed 5 months ago

MehfoozurRehman commented 5 months ago

so assets consume less bandwidth.

crandmck commented 5 months ago

Many of these assets are no longer used due to landing https://github.com/expressjs/expressjs.com/pull/1484. But some (e.g. the Express logo) are still used.

Using less bandwidth is good, so I'm inclined to land it, but can anyone speak to the specific details of the changes in this PR?

NOTE: We also need a PR to remove all the company logos that used to be on the "Companies using Express" page removed in the above-mentioned PR. Any takers?

chrisdel101 commented 5 months ago

What do you need removed? I can do it. Do I need to wait until this PR #1489 is merged before doing it?

crandmck commented 5 months ago

Some of the files modified in this PR are no longer used. For example, I believe that the files in images/companies are no longer used anywhere since #1484 landed.

So modifying them doesn't serve any purpose. How about removing them in this PR, then we can land it?
Ideally, we would have some way of determining ALL the images that are no longer used, and just remove them, but that might be asking too much :-)

MehfoozurRehman commented 5 months ago

ok i am on it.

chrisdel101 commented 5 months ago

Sorry, thought you were asking for a diff PR for a separate issue. If this is the case I can do that. But don't want to interfere with this current PR.