Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/5335#issuecomment-91691241
I tend to trust your hand on this. This is a big change and it's hard to
match up the existing logic with new logic in the diff, though it looks like
that was the intent and effect from spot-checking some elements. That the tests
have passed several times is a good sign. The reorganization is significantly
positive since the fields, and their initialization, are clearly grouped now.
One minor style thing, why the `_member` naming syntax, where it's not
otherwise used in Spark? just to make it crystal clear here what's a member?
I think the additional changes look reasonable, like using a Java
`Executor` in the, well, `Executor`.
I favor this though weakly on the grounds that I'm mostly relying on tests
for correctness. The intent is sound. @rxin @pwendell do you have any thoughts
on this one?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
