Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eliminate dev().assert() and dev().fine() calls from src/log.js module #4271

Closed

Conversation

erwinmombay
Copy link
Member

@erwinmombay erwinmombay commented Jul 29, 2016

This is in preparation for the changes @jridgewell is doing for us to tree shake the log module better. (remove side effects from a simple module import)

@erwinmombay erwinmombay force-pushed the eliminate-new-log-calls branch 2 times, most recently from 7e4f9cb to b5f441d Compare July 29, 2016 10:48
@erwinmombay
Copy link
Member Author

cc @jridgewell just need to add tests

@erwinmombay erwinmombay force-pushed the eliminate-new-log-calls branch from b5f441d to e5dbf4d Compare July 29, 2016 10:50
@erwinmombay
Copy link
Member Author

will eventually remove the old dev.assert()/dev.fine() removal code

@erwinmombay erwinmombay force-pushed the eliminate-new-log-calls branch from e5dbf4d to 28e9af3 Compare July 29, 2016 11:09
@jridgewell
Copy link
Contributor

LGTM. I'm going to merge this into my branch.

@erwinmombay erwinmombay force-pushed the eliminate-new-log-calls branch 2 times, most recently from 0beed9d to dd5b16e Compare July 29, 2016 16:01
@erwinmombay erwinmombay changed the title [WIP] eliminate dev().assert() and dev().fine() calls from src/log.js module eliminate dev().assert() and dev().fine() calls from src/log.js module Jul 29, 2016
@erwinmombay
Copy link
Member Author

tests added @jridgewell let me know when you have your changes pushed to a PR so i can test against it. thanks

@erwinmombay erwinmombay force-pushed the eliminate-new-log-calls branch from dd5b16e to e4a0f12 Compare July 29, 2016 16:07
@jridgewell
Copy link
Contributor

I can confirm they DCE correctly.

@jridgewell
Copy link
Contributor

Closed by #4261.

@jridgewell jridgewell closed this Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants