search

stamen / modestmaps-js

Modest Maps javascript port
http://modestmaps.com
566 stars 152 forks source link

Replacing alert with console.log for future node.js compatibility #25

Closed tmcw closed 13 years ago

tmcw commented 13 years ago

alert() is one of the few cases of browser-only code in nodejs. This pull replaces the three instances of alert with safeguarded console.log() calls.

RandomEtc commented 13 years ago

Oh hi! Welcome aboard. Quick on the draw!

Could you update CHANGELOG if you bump the version number?

Maybe these cases should throw new Error('foo') rather than console.log, since console doesn't exist in all browsers.

tmcw commented 13 years ago

Good points - I was going to do a build first... the builds weren't reflecting the v0.1.6 bump that happened before, so that's just a result of rebuilding.

Good point on exceptions instead of console log. I'll make that change.

RandomEtc commented 13 years ago

Ah yes, I see how it got mismatched, sorry about that. I'd bump to 0.16.1 for these error changes - not an API change or a feature change really, but good to keep track.

tmcw commented 13 years ago

Cool - bumped to 0.16.1, and replaced console.log with exceptions.