Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/42#issuecomment-37776148
Hey @andrewor14 I took another pass on this. Things are looking good but a
few notes in the code. In a few places it seemed like `Option` was used in a
way that causes what are really runtime failures to be ignored. It's better in
these cases to actually cause an exception in less there is a legitimate reason
the option could be `None`. Otherwise if there are failures they are really
hard to understand/debug.
There are a couple of things in here that I was a bit confused by and we
can discuss offline. They were the special case for the BlockManager listener
(I still wonder if we can come up with a better solution than is currently
there) and the use of redirection for static code.
In general though this is looking good and I think it's close to being
ready to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
