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.

Sign up for GitHub

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

Jump to bottom

[Spark QA] only check code files for new classes #2184

Closed
wants to merge 3 commits into from
Closed

[Spark QA] only check code files for new classes #2184

wants to merge 3 commits into from

Conversation

nchammas
Copy link
Contributor

Look only at code files (.py, .java, and .scala) for new classes.

Should get rid of false alarms like the one reported here.

@nchammas
Copy link
Contributor Author

@liancheng You may be interested in taking a look at this PR, too. (related to #2140)

@nchammas
Copy link
Contributor Author

Jenkins, retest this please.

@nchammas
Copy link
Contributor Author

The previous test cycle timed out on fetching the code.

@SparkQA
Copy link

SparkQA commented Aug 28, 2014

QA tests have started for PR 2184 at commit 9e2c721.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 28, 2014

QA tests have finished for PR 2184 at commit 9e2c721.

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

@nchammas
Copy link
Contributor Author

Another random failure that I don't think is related to this patch.

@nchammas
Copy link
Contributor Author

Jenkinmensch, retest this please.

liancheng
liancheng reviewed Aug 28, 2014
View reviewed changes
dev/run-tests-jenkins
@@ -93,7 +91,12 @@ function post_message () {
else
merge_note=" * This patch merges cleanly."

non_test_files=$(git diff master --name-only | grep -v "\/test" | tr "\n" " ")
non_test_files=$(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can rename it to source_files now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@liancheng
Copy link
Contributor

Jenkins, test this please.

@liancheng
Copy link
Contributor

LGTM, thanks!

@SparkQA
Copy link

SparkQA commented Aug 28, 2014

QA tests have started for PR 2184 at commit 9e2c721.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 28, 2014

QA tests have finished for PR 2184 at commit 9e2c721.

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

@SparkQA
Copy link

SparkQA commented Aug 29, 2014

QA tests have started for PR 2184 at commit 14e2335.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 29, 2014

QA tests have finished for PR 2184 at commit 14e2335.

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

@nchammas nchammas mentioned this pull request Aug 30, 2014
Closed
@nchammas
Copy link
Contributor Author

@pwendell I think this PR is ready for a final review / merging.

pwendell
pwendell reviewed Aug 30, 2014
View reviewed changes
dev/run-tests-jenkins
@@ -138,7 +141,7 @@ function post_message () {
test_result="$?"

if [ "$test_result" -eq "124" ]; then
fail_message="**Tests timed out** after a configured wait of \`${TESTS_TIMEOUT}\`."
fail_message="**[Tests timed out](${BUILD_URL}consoleFull)** after a configured wait of \`${TESTS_TIMEOUT}\`."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100 characters... can we break this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, done.

@pwendell
Copy link
Contributor

Minor style note, but otherwise LGTM

@SparkQA
Copy link

SparkQA commented Aug 30, 2014

QA tests have started for PR 2184 at commit 638c0e4.

  • This patch does not merge cleanly!

@nchammas
Copy link
Contributor Author

Hmm, looks like I need to fix something now that this doesn't merge cleanly anymore. Investigating.

@SparkQA
Copy link

SparkQA commented Aug 30, 2014

QA tests have finished for PR 2184 at commit 638c0e4.

  • This patch passes unit tests.
  • This patch does not merge cleanly!

@nchammas
[Spark QA] only check code files for new classes
8b82a26 
Look only at code files (py, java, and scala) for new classes.

Should get rid of false alarms like [the one reported
here](#2014 (comment)).
@nchammas
rename array of source files
3f91a14
@nchammas
break up long line
33786ac 
Thankfully, the extra whitespace gets removed when the output is parsed
as Markdown.
@nchammas
Copy link
Contributor Author

@pwendell I think we're all set now.

@SparkQA
Copy link

SparkQA commented Aug 30, 2014

QA tests have started for PR 2184 at commit 33786ac.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 30, 2014

QA tests have finished for PR 2184 at commit 33786ac.

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

@pwendell
Copy link
Contributor

Cool - thanks Nick!

@asfgit asfgit closed this in c567a68 Aug 31, 2014
@nchammas nchammas deleted the jenkins-ignore-noncode branch August 31, 2014 18:27
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
@nchammas @conviva-zz
[Spark QA] only check code files for new classes
abaa715 
Look only at code files (`.py`, `.java`, and `.scala`) for new classes.

Should get rid of false alarms like [the one reported here](apache#2014 (comment)).

Author: Nicholas Chammas <[email protected]>

Closes apache#2184 from nchammas/jenkins-ignore-noncode and squashes the following commits:

33786ac [Nicholas Chammas] break up long line
3f91a14 [Nicholas Chammas] rename array of source files
8b82a26 [Nicholas Chammas] [Spark QA] only check code files for new classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nchammas @SparkQA @liancheng @pwendell