Skip to content
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

[SPARK-10247] [core] improve readability of a test case in DAGSchedulerSuite #8434

Closed
wants to merge 3 commits into from

Conversation

squito
Copy link
Contributor

@squito squito commented Aug 25, 2015

This is pretty minor, just trying to improve the readability of DAGSchedulerSuite, I figure every bit helps. Before whenever I read this test, I never knew what "should work" and "should be ignored" really meant -- this adds some asserts & updates comments to make it more clear. Also some reformatting per a suggestion from @markhamstra on #7699

@rxin
Copy link
Contributor

rxin commented Aug 25, 2015

Can we add some blank line to separate different checks?

@SparkQA
Copy link

SparkQA commented Aug 25, 2015

Test build #41561 timed out for PR 8434 at commit 8e2a969 after a configured wait of 175m.

@andrewor14
Copy link
Contributor

retest this please

@andrewor14
Copy link
Contributor

@squito can you add the suggested blank lines? Otherwise this LGTM.

@SparkQA
Copy link

SparkQA commented Sep 1, 2015

Test build #41890 has finished for PR 8434 at commit 8e2a969.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@squito
Copy link
Contributor Author

squito commented Sep 2, 2015

@andrewor14 added blank lines (and also expanded comments a teeny bit)

@SparkQA
Copy link

SparkQA commented Sep 2, 2015

Test build #41935 has finished for PR 8434 at commit 94927c8.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Sep 2, 2015

Test build #41942 has finished for PR 8434 at commit 94927c8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

Merged into master, thanks.

@asfgit asfgit closed this in 3ddb9b3 Sep 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants