cgruberatgoog commented on revision r1191 in project google-guice.
Details are at http://code.google.com/p/google-guice/source/detail?r=1191
Score: Positive
General Comment:
Wonderful.
Line-by-line comments:
File: /trunk/src/com/google/inject/internal/Errors.java (r1191)
===============================================================================
Line 324: return errorInUserCode(unwrapped, "Error in custom
provider, %s", unwrapped);
-------------------------------------------------------------------------------
Yeah, I think this is much easier to consume.
Line 370: if(runtimeException instanceof
Exceptions.UnhandledCheckedUserException) {
-------------------------------------------------------------------------------
Really? Shouldn't this roll all the way to getCause() == null? ... as a
developer looking at the underlying framework throwing an exception, I'd
rather see the root cause -the app exception can be used for catching and
app-level handling. I think here it's more valuable to see the NPE, not
the exception I wrapped it in. having said that, I'm 50/50 on that or what
you've done. This is a marked improvement.
Respond to these comments at
http://code.google.com/p/google-guice/source/detail?r=1191
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en.
