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

Merged
merged 1 commit into from May 3, 2011

Conversation

Projects
None yet
2 participants
Contributor

tmcw commented May 3, 2011

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.

@tmcw tmcw added a commit that referenced this pull request May 3, 2011

@tmcw tmcw Merge pull request #25 from tmcw/serverside.
Replacing alert with console.log for future node.js compatibility
38456fb

@tmcw tmcw merged commit 38456fb into stamen:master May 3, 2011

Contributor

RandomEtc commented May 3, 2011

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.

Contributor

tmcw commented May 3, 2011

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.

Contributor

RandomEtc commented May 3, 2011

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.

Contributor

tmcw commented May 9, 2011

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment