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-22103][FOLLOWUP] Rename addExtraCode to addInnerClass #19353

Closed

Conversation

juliuszsompolski
Copy link
Contributor

What changes were proposed in this pull request?

Address PR comments that appeared post-merge, to rename addExtraCode to addInnerClass,
and not count the size of the inner class to the size of the outer class.

How was this patch tested?

YOLO.

@juliuszsompolski
Copy link
Contributor Author

@cloud-fan @viirya I addressed the post-merge comments you had in #19324

@@ -335,16 +335,15 @@ class CodegenContext {
* Emits any source code added with addExtraCode
Copy link
Member

Choose a reason for hiding this comment

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

nit: Emits extra classes added ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Emits extra inner classes added ...

@viirya
Copy link
Member

viirya commented Sep 26, 2017

LGTM

1 similar comment
@cloud-fan
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Sep 26, 2017

Test build #82197 has finished for PR 19353 at commit 55104df.

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

@juliuszsompolski
Copy link
Contributor Author

jenkins retest this please

@SparkQA
Copy link

SparkQA commented Sep 26, 2017

Test build #82199 has finished for PR 19353 at commit c0488fb.

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

@gatorsmile
Copy link
Member

Thanks! Merged to master.

@asfgit asfgit closed this in f21f6ce Sep 26, 2017
@SparkQA
Copy link

SparkQA commented Sep 26, 2017

Test build #82204 has finished for PR 19353 at commit c0488fb.

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

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.

5 participants