-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Fancier logging! #3
Conversation
…nd the module identifier that is requested via requirejs.
I like it! Can you fix the build failure (JSHint issue)? |
nice |
@@ -113,13 +113,13 @@ define("resolver", | |||
} | |||
|
|||
if (Ember.ENV.LOG_MODULE_RESOLVER) { | |||
Ember.Logger.info('hit', moduleName); | |||
Ember.Logger.info('[*]', parsedName.fullName, Array(40 - parsedName.fullName.length).join('.'), moduleName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use: ✓ ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that.
Given Ember code standards, can we use ✓ (the unicode character) in the code or should we use String.fromCharCode(0x2713)
?
Possibly out of scope, but I'd love to start adding tests for future changes. In this case we could just test that hit and miss return the expected value. |
Looks great to me :) |
thoughts on my unicode checkmark proposal? |
I like the idea of using the checkmark. |
✓ should be fine, unless their exists an issue i am aware of. |
👍 |
Add license and barebones README.
Display both fullname as passed into the resolver and the module identifier that is requested via requirejs.
I think this makes it much easier to understand what the resolver is doing.
What do you think?