[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-23 Thread jaceklaskowski
Github user jaceklaskowski closed the pull request at: https://github.com/apache/spark/pull/10433 --- 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

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166701682 Your debate aside, I am not sure whether it makes sense to change the current one. It has virtually zero impact on anything, and as a result it is more or less changing

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread markhamstra
Github user markhamstra commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166713847 More substantively, `if...else` generally performs better than pattern matching, and this is a fairly hot code path. --- If your project is set up for it, you can

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166616317 I think the logic is equivalent, yes, but it isn't any shorter or clearer. Yes, the issue is that it looks strange to check if the state is FINISHED and then otherwise

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166615564 Can one of the admins verify this patch? --- 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

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread jaceklaskowski
GitHub user jaceklaskowski opened a pull request: https://github.com/apache/spark/pull/10433 [CORE] Refactoring: Use pattern matching and dedicated method It should be slightly easier to see what really happens (without questions like what other state a task can be here).

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread jaceklaskowski
Github user jaceklaskowski commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166618819 I disagree since Spark core is hard due to what it does and making it harder can hinder contributions. There are just such little things that can change how

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166630101 I'm not suggesting any changes at all. If there were a few places in the code that call for checking for a state that is "finished, but not FINISHED" then I'd say it's

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166622804 The question is whether this makes the code more 'welcoming' or not, of course, and I don't think this does. I did not agree that the current logic is convoluted; I'm

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread jaceklaskowski
Github user jaceklaskowski commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166628562 Doh, you're *again* inviting me to make more involved changes (which is great, but don't think helps others to contribute). What I'm gonna do is to improve the

[GitHub] spark pull request: [CORE] Refactoring: Use pattern matching and d...

2015-12-22 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/10433#issuecomment-166813289 @jaceklaskowski can you close the pr? --- 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