search

erkie / erkie.github.com

Adding asteroids to any site on the web
http://erkie.github.com/
461 stars 92 forks source link

Drop unused functions / variables #23

Closed avindra closed 8 years ago

avindra commented 8 years ago

Noticed these weren't being used.

P.S., would you consider merging changes to drop IE8 support?

erkie commented 8 years ago

@avindra do these changes drop IE8 support or what do you mean?

avindra commented 8 years ago

@erkie these changes have nothing to do with ie8, its just dead code that I wanted to remove.

The P.S. was meant as a question to you--- if I made another PR which dropped IE8 support, would you be interested in merging it?

erkie commented 8 years ago

I'm not actively working on this project at all as the latest version is on http://www.kickassapp.com, so I don't really see a reason for deprecating IE8 if it is working. What would be the benefit?

avindra commented 8 years ago

One of my coworkers pulled this code into our project as an easter egg and I just had an urge to clean up the code :haircut:

If you're not interested thats fine, let me know and close the PR

erkie commented 8 years ago

I don't think we should remove browser support for the sake of removing support. But thanks for this PR 😄

avindra commented 8 years ago

Thanks. My intention isn't really just to remove support for the sake of it. IE8 is officially deprecated by Microsoft, so attachEvent hacks are a thing of a bygone era. addEventListener works natively in every single browser today.

My coworker actually checked in the code to our local git repo, and by removing the IE8 specific hacks, it went from 1126 lines to 925 lines. Can probably still use further cleanup too.